[FFmpeg-devel] [PATCH] avformat/utils: use the existing packet reference when parsing complete frames

[James Almer]

On 4/12/2018 3:59 PM, wm4 wrote:
> On Thu, 12 Apr 2018 15:34:29 -0300
> James Almer <jamrial at gmail.com> wrote:
> 
>> If the parser returns full frames, then the output pointer retured by
>> av_parser_parse2() is guaranteed to point to data contained in the
>> input packet's buffer.
>>
>> Create a new reference to said buffer in that case, to avoid
>> unnecessary data copy when queueing the packet later in the function.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavformat/utils.c | 23 ++++++++++++++++++++---
>>  1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 3e482a3bbc..8ad2ef4d70 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -1471,6 +1471,22 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, int stream_index)
>>          if (!out_pkt.size)
>>              continue;
>>  
>> +        if (pkt->buf && out_pkt.data == pkt->data) {
>> +            /* reference pkt->buf only when out_pkt.data is guaranteed to point
>> +             * to data in it and not in the parser's internal buffer. */
>> +            /* XXX: Ensure this is the case with all parsers when the muxer sets
>> +             * PARSER_FLAG_COMPLETE_FRAMES and check for that instead? */
>> +            out_pkt.buf = av_buffer_ref(pkt->buf);
>> +            if (!out_pkt.buf) {
>> +                ret = AVERROR(ENOMEM);
>> +                goto fail;
>> +            }
>> +        } else {
>> +            ret = av_packet_make_refcounted(&out_pkt);
>> +            if (ret < 0)
>> +                goto fail;
>> +        }
>> +
>>          if (pkt->side_data) {
>>              out_pkt.side_data       = pkt->side_data;
>>              out_pkt.side_data_elems = pkt->side_data_elems;
>> @@ -1511,10 +1527,11 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt, int stream_index)
>>  
>>          ret = ff_packet_list_put(&s->internal->parse_queue,
>>                                   &s->internal->parse_queue_end,
>> -                                 &out_pkt, FF_PACKETLIST_FLAG_REF_PACKET);
>> -        av_packet_unref(&out_pkt);
>> -        if (ret < 0)
>> +                                 &out_pkt, 0);
>> +        if (ret < 0) {
>> +            av_packet_unref(&out_pkt);
>>              goto fail;
>> +        }
>>      }
>>  
>>      /* end of the stream => close and free the parser */
> 
> For which cases is this?

Pretty much every single packet from a demuxer that sets
AVSTREAM_PARSE_HEADERS, meaning every non-raw demuxer with a couple
exceptions.

> Maybe we should just make a new parser API (or
> reuse the BSF one), and make it cover the cases we care about.

I remember a new parser API that works on packets rather than raw data,
much like the new bsf API, was the plan at some point.
There's no lack of ideas in any case, just manpower in general.