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

[wm4]

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? Maybe we should just make a new parser API (or
reuse the BSF one), and make it cover the cases we care about.