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

Removed Hz from audio_fps match in ffmpeg_parse_infos #665

Merged
merged 3 commits into from
Apr 23, 2018

Conversation

qmac
Copy link
Contributor

@qmac qmac commented Nov 14, 2017

Cuts the ' Hz' off of the regex match when trying to find the fps info of an audio stream.

Previously, this was always setting result['audio_fps'] to 'unknown' because the casting to an integer would fail.

@coveralls
Copy link

coveralls commented Nov 14, 2017

Coverage Status

Coverage decreased (-0.07%) to 57.133% when pulling 345d67c on qmac:master into a744df1 on Zulko:master.

@coveralls
Copy link

coveralls commented Nov 16, 2017

Coverage Status

Coverage decreased (-0.03%) to 57.172% when pulling 4a51503 on qmac:master into a744df1 on Zulko:master.

@qmac
Copy link
Contributor Author

qmac commented Nov 16, 2017

Coverage is only decreasing because https://github.com/Zulko/moviepy/pull/665/files#diff-a49583100a493b6fe1c05b48824e4a3cL389 is no longer executed on any of the test media files.

@tburrows13 tburrows13 added the bug-fix For PRs and issues solving bugs. label Nov 28, 2017
@tburrows13
Copy link
Collaborator

Thank you very much for this! I'll merge it right away

@tburrows13 tburrows13 self-assigned this Apr 23, 2018
@tburrows13
Copy link
Collaborator

Haha, just found this PR again. I've investigated, and you're definitely right, this is a bug. However, it doesn't appear that this ['audio_fps'] is used again by moviepy. Maybe it should be?

@tburrows13 tburrows13 merged commit 956c462 into Zulko:master Apr 23, 2018
@qmac
Copy link
Contributor Author

qmac commented Apr 23, 2018

Yes that may be true. Seems like VideoFileClips get the fps from the meta-info, but AudioFileClips rely on a user-provided fps, defaulting to 44100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix For PRs and issues solving bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants