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

Fix FPS setting on FFmpeg 4 #520

Merged
merged 3 commits into from
Jun 2, 2020
Merged

Fix FPS setting on FFmpeg 4 #520

merged 3 commits into from
Jun 2, 2020

Conversation

jonoomph
Copy link
Member

@jonoomph jonoomph commented Jun 1, 2020

Fix FPS setting on FFmpeg 4, which currently is not setting a valid FPS. This causes all sorts of strange behavior on videos produced with libopenshot using FFmpeg 4. This patch does require a depreciated approach, and I'm not sure how we are supposed to be correctly setting the FPS parameter, so I'm open to suggestions.

Also, thanks to PeterM for troubleshooting this issue and finding a solution! Nice work!

@codecov
Copy link

codecov bot commented Jun 1, 2020

Codecov Report

Merging #520 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #520   +/-   ##
========================================
  Coverage    48.33%   48.33%           
========================================
  Files          128      128           
  Lines         9962     9962           
========================================
  Hits          4815     4815           
  Misses        5147     5147           
Impacted Files Coverage Δ
src/FFmpegWriter.cpp 63.08% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10ed4ba...7ed26cf. Read the comment docs.

@jonoomph
Copy link
Member Author

jonoomph commented Jun 1, 2020

@ferdnyc Travis is having trouble with finding our FFmpeg PPA: https://launchpad.net/~jon-hedgerows/+archive/ubuntu/ffmpeg-backports. This all of a sudden does not seem to exist anymore. Ughhh.... 😉

@ferdnyc
Copy link
Contributor

ferdnyc commented Jun 1, 2020

Oy, not again. I'm beginning to understand why some people say they won't ever use community PPAs in production builds.

@ferdnyc
Copy link
Contributor

ferdnyc commented Jun 1, 2020

I'll just set the 3.2 build as allowed-to-fail, for now, like I did with FFmpeg 4 during the "walkout". But it looks like that PPA was made private, I doubt it's coming back. And there was no Ubuntu LTS release that included FFmpeg 3.2, it got skipped over. We may just have to drop that version from our build matrix.

@jonoomph
Copy link
Member Author

jonoomph commented Jun 1, 2020

I suppose we could always setup our own PPA builds, but that sounds like a huge amount of time to get things all automated and building constantly, and then we'll have the same problem that Jon has, a ton of other projects will see our PPA and start incorporating it into their production builds.

@ferdnyc
Copy link
Contributor

ferdnyc commented Jun 1, 2020

Fixed in your branch, and I'll open a PR for develop.

@jonoomph
Copy link
Member Author

jonoomph commented Jun 1, 2020

I'll just set the 3.2 build as allowed-to-fail, for now, like I did with FFmpeg 4 during the "walkout". But it looks like that PPA was made private, I doubt it's coming back. And there was no Ubuntu LTS release that included FFmpeg 3.2, it got skipped over. We may just have to drop that version from our build matrix.

Thanks, that makes sense!

@ferdnyc
Copy link
Contributor

ferdnyc commented Jun 1, 2020

OK, let's try that again.

@ferdnyc
Copy link
Contributor

ferdnyc commented Jun 1, 2020

There we go, now I see it's marked "allowed to fail'. I tried to mask it by environment variable like I did last time, but that didn't work. I think to do that I'd have to mask the entire environment config, and we have three variables now. So I masked it by name insead.

src/FFmpegWriter.cpp Outdated Show resolved Hide resolved
Co-authored-by: Frank Dana <ferdnyc@gmail.com>
@jonoomph jonoomph merged commit 1cdb49f into develop Jun 2, 2020
@jonoomph jonoomph deleted the fix-fps-ffmpeg4 branch June 2, 2020 19:28
@SuslikV
Copy link
Contributor

SuslikV commented Jun 3, 2020

...This causes all sorts of strange behavior on videos produced with libopenshot using FFmpeg 4.

This should include description of the problem.

The fix itself is incorrect. And someone told me that will use the FFmpegUtilities.h in such cases. What's up man?

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.

None yet

3 participants