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

WIP: Patches to let libopenshot compile with ffmpeg version after 4.4 #670

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

eisneinechse
Copy link
Collaborator

This is a work in progress. The export of the audio is still broken
which started on June 18 with the removal of deprecated call in
ffmpeg.

This is a work in progress. The export of the audio is still broken
which started on June 18 with the removal of deprecated call in
ffmpeg.
@codecov
Copy link

codecov bot commented May 2, 2021

Codecov Report

Merging #670 (fd6a016) into develop (68f03b5) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #670   +/-   ##
========================================
  Coverage    50.41%   50.41%           
========================================
  Files          155      155           
  Lines        13315    13315           
========================================
  Hits          6713     6713           
  Misses        6602     6602           
Impacted Files Coverage Δ
src/FFmpegUtilities.h 100.00% <ø> (ø)
src/FFmpegWriter.h 25.00% <ø> (ø)
src/FFmpegReader.cpp 68.35% <100.00%> (ø)
src/FFmpegWriter.cpp 59.67% <100.00%> (ø)

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 68f03b5...fd6a016. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jun 5, 2021

Merge conflicts have been detected on this PR, please resolve.

@github-actions github-actions bot added the conflicts A PR with unresolved merge conflicts label Jun 5, 2021
@eisneinechse eisneinechse requested a review from ferdnyc June 14, 2021 02:13
@eisneinechse eisneinechse changed the title (WIP) Patches to let libopenshot compile with ffmpeg 4.4 (WIP) Patches to let libopenshot compile with ffmpeg version after 4.4 Jun 14, 2021
@github-actions github-actions bot added conflicts A PR with unresolved merge conflicts and removed conflicts A PR with unresolved merge conflicts labels Jun 25, 2021
@github-actions
Copy link

Merge conflicts have been detected on this PR, please resolve.

@ferdnyc ferdnyc added ffmpeg Issues or PRs involving the a/v processing code and removed conflicts A PR with unresolved merge conflicts labels Jul 3, 2021
@github-actions github-actions bot added the conflicts A PR with unresolved merge conflicts label Dec 2, 2021
@github-actions
Copy link

github-actions bot commented Dec 2, 2021

Merge conflicts have been detected on this PR, please resolve.

Comment on lines 2062 to 2066
#if IS_FFMPEG_4_4
pkt.size = sizeof(AVFrame);
#else
pkt.size = sizeof(AVPicture);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah! Well, crap. I only just noticed that this 4-month-old comment was still "Pending", and only visible to me! Whoops. Sorry about that.


Instead of directly setting packet.size, should this be using av_packet_from_data(&pkt, buffer, size)?

Or, in the cases where the data isn't in an av_malloc()'d buffer like _from_data() requires (i.e. audio), at the very least an av_new_packet(&pkt, size) followed by a memcpy()?

av_init_packet() has now been deprecated, a change that was formally made only recently... but in part, it's been deprecated because there have been better replacements for a very long time. Both av_new_packet() and av_packet_from_data() were available as far back as FFmpeg 2.4. So, we could switch our existing code away from it for all FFmpeg versions, not just 4.4+.


...Ultimately that's what I ended up doing in #784, since av_packet_from_data has "always" (as far as we're concerned) been available. So now we just leave it to avcodec (or is it avformat? avutil?) to figure out how to size things. I figure it should know best!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, no. That change was in #809.

@jonoomph jonoomph changed the title (WIP) Patches to let libopenshot compile with ffmpeg version after 4.4 WIP: Patches to let libopenshot compile with ffmpeg version after 4.4 Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts A PR with unresolved merge conflicts ffmpeg Issues or PRs involving the a/v processing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants