[FFmpeg-devel] [PATCH v3] avformat/mux: Make uncoded frames av_packet_unref() compatible

Marton Balint cus at passwd.hu
Thu Apr 16 23:09:33 EEST 2020



On Tue, 14 Apr 2020, Nicolas George wrote:

> Andreas Rheinhardt (12020-04-12):
>> Currently uncoded frames (i.e. packets whose data actually points to an
>> AVFrame) are not refcounted. As a consequence, calling av_packet_unref()
>> on them will not free them, but may simply make sure that they leak by
>> losing the pointer to the frame.
>>
>> This commit changes this by actually making uncoded frames refcounted.
>> In order not to rely on sizeof(AVFrame) (which is not part of the public
>> API and so must not be used here in libavformat) the packet's data is
>> changed to a (padded) buffer containing just a pointer to an AVFrame.
>> Said buffer is owned by an AVBuffer with a custom free function that
>> frees the frame as well as the buffer. Thereby the pointer/the AVBuffer
>> owns the AVFrame.
>>
>> Said ownership can actually be transferred by copying and resetting
>> the pointer, as might happen when actually writing the uncoded frames
>> in AVOutputFormat.write_uncoded_frame() (although currently no muxer
>> makes use of this possibility).
>>
>> This makes packets containing uncoded frames compatible with
>> av_packet_unref(). This already has three advantages in interleaved mode:
>> 1. If an error happens at the preparatory steps (before the packet is
>> put into the interleavement queue), the frame is properly freed.
>> 2. If the trailer is never written, the frames still in the
>> interleavement queue will now be properly freed by
>> ff_packet_list_free().
>> 3. The custom code for moving the packet to the packet list in
>> ff_interleave_add_packet() can be removed.
>>
>> It will also simplify fixing further memleaks in future commits.
>>
>> Suggested-by: Marton Balint <cus at passwd.hu>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>> How embarrassing! The earlier version forgot to check the allocation.
>
> I am confused: does it not make unwrapped frames behave exactly the same
> as wrapped frames?

There is a slight difference, for WRAPPED_AVFRAME packets the AVFrame is 
stored in the data, for uncoded_frames packets only a pointer to AVFrame 
is stored in data.

>
> AFAIU, Marton intends to remove all this code, and I think he is right,
> because it was a hack.

Yes, but full removal can only happen at the next bump, so until that this 
should be fixed. Also this patchset has been delayed for a very long time 
now, I really like to see it applied as soon as possible, even if we 
further change uncoded_frames support later or around the time of the API 
bump.

Thanks,
Marton


More information about the ffmpeg-devel mailing list