[FFmpeg-devel] [PATCH 2/3] avfilter/src_movie: Remove unnecessary secondary AVPacket

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Thu Sep 10 15:05:20 EEST 2020


Nicolas George:
> Andreas Rheinhardt (12020-09-10):
>> The movie and amovie filters currently use two packets. One of the two,
>> pkt0, is the owner of the returned packet; it is also the destination
>> packet for av_read_frame(). The other one pkt is initially (i.e. after
>> av_read_frame()) a copy of pkt0; copy means that the contents of both
>> are absolutely the same: They both point to the same AVBufferRef and the
>> same side data. This violation of the refcounted packet API is only
>> possible because pkt is not considered to own its data. Only pkt0 is
>> ever unreferenced.
>> The reason for pkt's existence seems to be historic:
>> The API used for decoding audio (namely avcodec_decode_audio4()) could
>> consume frames partially, i.e. it could return multiple frames for one
>> packet and therefore it returned how much of the input buffer had been
>> consumed. The caller was then supposed to update the packet's data and
>> size pointer to reflect this and call avcodec_decode_audio4() again with
>> the updated packet to get the next frame.
>> But before the introduction of refcounted AVPackets where knowledge and
>> responsibility about what to free lies with the underlying AVBuffer such
>> a procedure required a spare packet (or one would need to record the
>> original data and size fields separately to restore them before freeing
>> the packet; notice that this code has been written when AVPackets still
>> had a destruct field). But these times are long gone, so just remove the
>> secondary AVPacket.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>> It seems that the avcodec_decode_audio4() lost the ability to output
>> multiple frames per packet in 061a0c14bb5767bca72e3a7227ca400de439ba09.
>> Am I right?
> 
> I think I am ok with the change (and the two others), but I think it
> would be much more useful update src_movie to use the new decoding API:
> no need to keep a packet in the context, no need to distinguish between
> audio and video.
> 
> More work, but more useful. And it would allow to properly implement an
> activate version.
> 
> Regards,
> 
I might implement the switch to the new decoding API later, but I wanted
to fix the memleak first.

- Andreas


More information about the ffmpeg-devel mailing list