[FFmpeg-devel] [PATCH v4 7/9] avformat/utils: Avoid copying packets unnecessarily

[Andreas Rheinhardt]

James Almer:
> On 9/20/2019 5:39 PM, Andreas Rheinhardt wrote:
>> Up until now, read_frame_internal in avformat/utils.c uses a spare
>> packet on the stack that serves no real purpose: At no point in this
>> function is there a need for another packet besides the packet destined
>> for output:
>> 1. If the packet doesn't need a parser, but is output as is, the content
>> of the spare packet (that at this point contains a freshly read packet)
>> is simply copied into the output packet (via simple assignment, not
>> av_packet_move_ref, thereby confusing ownership).
>> 2. If the packet needs parsing, the spare packet will be reset after
>> parsing and any packets resulting from the packet read will be put into
>> a packet list; the output packet is not used here at all.
>> 3. If the stream should be discarded, the spare packet will be
>> unreferenced; the output packet is not used here at all either.
>>
>> Therefore the spare packet and the copies can be removed in principle.
>> In practice, one more thing needs to be taken care of: If ff_read_packet
>> failed, the output packet was not affected, now it is. But given that
>> ff_read_packet returns a blank (as if reset via av_packet_unref) packet
>> on failure, there is no problem from this side either.
> 
> There's still the "if (!pktl && st->request_probe <= 0)" check in
> ff_read_packet(), which returns without unreferencing the packet.
> 
And that's how it should be, because this is not failure. It is the
ordinary way to return from ff_read_packet() when reading was
successfull and the probing is unnecessary. In this case, there is no
need for the overhead of the packet list.

- Andreas