-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Audio preview fixes #2267
base: master
Are you sure you want to change the base?
Audio preview fixes #2267
Conversation
Hi, typo and fixes seems good but I'm not sure about the logfile (not sure PIPE is the right default output by the way) and the removing of some params from the ffplay command... |
ac is only for ffmpeg
I don't really know how to do pull requests. So please expect some weirdness... I split the doc fixes and code fixes to two requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this commit, there was obviously an issue. logfile was not defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you go to ffplay documentation, you will find that -ac
is not one of the options.
Hi, after checking -ac is actually a ffplay option at least up to ffplay version 5.1, so you would actually need to check ffplay version and use -ac option for version 5.* and maybe 6.* too, and migrate the equivalent in 7.*, which seems to be |
I was trying follow the docs and found several issues in the doc and code.
Please take a look.