[FFmpeg-devel] [PATCH 23/50] avformat/mux: use av_packet_alloc() to allocate packets

[James Almer]

On 2/8/2021 3:22 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer<jamrial at gmail.com>
>> ---
>>   libavformat/internal.h |  5 +++++
>>   libavformat/mux.c      | 44 ++++++++++++++++++++++--------------------
>>   libavformat/options.c  |  6 ++++++
>>   libavformat/utils.c    |  1 +
>>   4 files changed, 35 insertions(+), 21 deletions(-)
>>
>> diff --git a/libavformat/internal.h b/libavformat/internal.h
>> index d0db331b96..69a7caff93 100644
>> --- a/libavformat/internal.h
>> +++ b/libavformat/internal.h
>> @@ -92,6 +92,11 @@ struct AVFormatInternal {
>>        */
>>       struct AVPacketList *parse_queue;
>>       struct AVPacketList *parse_queue_end;
>> +
>> +    /**
>> +     * Used to hold temporary packets.
>> +     */
>> +    AVPacket *pkt;
>>       /**
>>        * Remaining size available for raw_packet_buffer, in bytes.
>>        */
>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> index 84c56ac6ba..3600e74a50 100644
>> --- a/libavformat/mux.c
>> +++ b/libavformat/mux.c
>> @@ -1052,7 +1052,9 @@ int ff_interleaved_peek(AVFormatContext *s, int stream,
>>       AVPacketList *pktl = s->internal->packet_buffer;
>>       while (pktl) {
>>           if (pktl->pkt.stream_index == stream) {
>> -            *pkt = pktl->pkt;
>> +            int ret = av_packet_ref(pkt, &pktl->pkt);
>> +            if (ret < 0)
>> +                return ret;
> This will lead to memleaks, because up until now the callers just
> received a non-ownership packets, ergo they did not unref the packet.
> (The fate-movenc test fails with this when run under valgrind/asan.)
> 
> Returning a pointer to a const AVPacket and signaling the offsetted
> timestamps via other pointer arguments would avoid this problem.

There's a single user of ff_interleaved_peek(), which is the movenc one, 
and i made it unref the packet right after using it. But that's done in 
the following patch, so i guess i should change this function and its 
one caller at the same time.

Also, i could just return the offsetted pts and dts values and not 
bother at all with packets. No allocations that way.