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

Switch to new FFmpeg AVPacket API #938

Merged
merged 4 commits into from
Jan 18, 2025

Conversation

complexlogic
Copy link
Contributor

@complexlogic complexlogic commented Jan 5, 2025

A significant breaking change is coming to the FFmpeg API in the next major version bump of libavcodec, which is expected to occur with the release of FFmpeg 8 sometime in the next few months. See #778 for more details.

  • sizeof(AVPacket) is being removed from the public ABI. The FFmpeg developers want to add fields to the end of the struct without bumping the major version. This means that calling applications should not allocate the AVPacket structure, but instead use av_packet_alloc to let FFmpeg allocate it. Otherwise, there is a risk of a segfault.
  • av_init_packet is being removed (av_packet_alloc takes over this functionality).
  • AVPacketList is being removed

New code is added to use the FFmpeg allocator for TAVPacket, and operate on PAVPacket instead of TAVPacket. A custom record type TPacketList was introduced to replace TAVPacketList, which fulfills the same purpose.

I've done some testing locally on both Linux and Windows. Everything is working on my end and I haven't detected any memory leaks.

Fixes #778

@s09bQ5
Copy link
Collaborator

s09bQ5 commented Jan 5, 2025

Resolving #939 first should further simplify the needed changes. The new API functions are supported since FFmpeg 3.0.

@complexlogic
Copy link
Contributor Author

@s09bQ5 I agree.

I would like to implement #939 in this PR if the maintainers are OK with that. It makes sense to me to combine them, so I don't have to test two large PRs separately. It would be nice to avoid the duplicated effort.

@s09bQ5
Copy link
Collaborator

s09bQ5 commented Jan 6, 2025

Looking at the stuff that will be removed with the support for old FFmpeg versions, I just noticed that our StatusInfo stuff is completely broken since ef52574 tried to fix it for FFmpeg 2.0. TPacketQueue.GetStatusInfo no longer returns a value, but the result is used in TFFmpegDecodeStream.DecodeFrame. Luckily there appear to be only few audio files that don't start at PTS 0. Can you add a field to TPacketList to store the StatusInfo there?

Edit: We don't need to do that. Since FFmpeg 5.0 there is an opaque element in AVPacket that could be used instead of priv. Or we abuse the side_data pointer. Storing StartSilence (i.e. the only kind of StatusInfo) in pts would be possible as well.

@complexlogic
Copy link
Contributor Author

I'm going to prioritize working on #939 first, and change this PR to draft. If/when the fix for #939 is merged into master, I'll rebase this PR.

@complexlogic complexlogic marked this pull request as draft January 6, 2025 14:00
@complexlogic
Copy link
Contributor Author

This has been rebased to current master and is ready for review. Since #944 has been merged, all supported versions of FFmpeg have the required new AVPacket API functions. So, I changed the code to use the new API only for simplicity.

@complexlogic complexlogic changed the title Support new FFmpeg AVPacket API Switch to new FFmpeg AVPacket API Jan 16, 2025
@complexlogic complexlogic marked this pull request as ready for review January 16, 2025 14:22
src/media/UAudioDecoder_FFmpeg.pas Outdated Show resolved Hide resolved
src/media/UAudioDecoder_FFmpeg.pas Show resolved Hide resolved
src/media/UMediaCore_FFmpeg.pas Outdated Show resolved Hide resolved
@bohning bohning merged commit 3b13eb3 into UltraStar-Deluxe:master Jan 18, 2025
5 checks passed
@complexlogic complexlogic deleted the ffmpeg8 branch January 18, 2025 17:25
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.

Prepare for size of struct AVPacket becoming private
3 participants