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

Support new FFmpeg AVPacket API #938

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

complexlogic
Copy link

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. The necessary changes were implemented for FFmpeg 7, and can be carried over for FFmpeg 8 when the time comes.

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
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
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
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
2 participants