[FFmpeg-devel] [PATCH v4 3/9] avformat/utils: Move the reference to the packet list

[Andreas Rheinhardt]

James Almer:
> On 9/20/2019 5:39 PM, Andreas Rheinhardt wrote:
>> Up until now, ff_packet_list_put had a flaw: When it moved a packet to
>> the list (meaning, when it ought to move the reference to the packet
>> list instead of creating a new one via av_packet_ref), it did not reset
>> the original packet, confusing the ownership of the data in the packet.
>> This has been done because some callers of this function were not
>> compatible with resetting the packet.
>>
>> This commit changes these callers and fixes this flaw. In order to
>> indicate that the ownership of the packet has moved to the packet list,
>> pointers to constant AVPackets are used whenever the target of the
>> pointer might already be owned by the packet list.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>>  libavformat/utils.c | 32 +++++++++++++++++---------------
>>  1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index e348df3269..58e0db61da 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -460,10 +460,7 @@ int ff_packet_list_put(AVPacketList **packet_buffer,
>>              return ret;
>>          }
>>      } else {
>> -        // TODO: Adapt callers in this file so the line below can use
>> -        //       av_packet_move_ref() to effectively move the reference
>> -        //       to the list.
>> -        pktl->pkt = *pkt;
>> +        av_packet_move_ref(&pktl->pkt, pkt);
> 
> I think it would be a good idea ensuring the packet is reference counted
> here, to have more robust code and not depend on current callers having
> done it themselves.
> It's a matter of adding an av_packet_make_refcounted() call before this
> line, much like it's done in av_bsf_send_packet(). It's essentially a
> no-op when the packet is already reference counted.
> 
I can add a commit doing this. But there is at least one potential
use-case where using av_packet_make_refcounted() is not good: Uncoded
frames. They aren't currently used in conjunction with this function,
but that may change of course. Should I add a check for this scenario
or another FF_PACKETLIST_FLAG_* flag that says that the packet should
not be made refcounted, but simply moved?

- Andreas