Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Audio preview fixes #2267

wants to merge 2 commits into from

Conversation

junhe
Copy link
Contributor

@junhe junhe commented Nov 29, 2024

I was trying follow the docs and found several issues in the doc and code.

Please take a look.

@OsaAjani
Copy link
Collaborator

OsaAjani commented Dec 4, 2024

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...
Would you remove those and eventually just write them as their own PR with some explanations on what the intent is ?

@junhe
Copy link
Contributor Author

junhe commented Dec 5, 2024

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.

Copy link
Contributor Author

@junhe junhe left a 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.

Copy link
Contributor Author

@junhe junhe left a 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.

@OsaAjani OsaAjani changed the title Doc typo fixes and video preview fixes Audio preview fixes Jan 3, 2025
@OsaAjani
Copy link
Collaborator

OsaAjani commented Jan 3, 2025

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 -ch_layout according to this ticket https://trac.ffmpeg.org/ticket/11077

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants