diff mbox series

[FFmpeg-devel,03/18] avformat/utils: Don't allocate separate packet for extract_extradata

Message ID 20210319055904.2264501-3-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,01/18] libavformat/utils: Fix indentation | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt March 19, 2021, 5:58 a.m. UTC
One can simply reuse AVFormatInternal.parse_pkt instead.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/internal.h | 12 ++++++++++--
 libavformat/utils.c    | 29 ++++++++++-------------------
 2 files changed, 20 insertions(+), 21 deletions(-)

Comments

James Almer March 23, 2021, 3:23 a.m. UTC | #1
On 3/19/2021 2:58 AM, Andreas Rheinhardt wrote:
> One can simply reuse AVFormatInternal.parse_pkt instead.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>   libavformat/internal.h | 12 ++++++++++--
>   libavformat/utils.c    | 29 ++++++++++-------------------
>   2 files changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 3c6b2921c1..a810d51bba 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -90,9 +90,18 @@ struct AVFormatInternal {
>       /**
>        * Packets split by the parser get queued here.
>        */
> -    AVPacket *parse_pkt;
>       struct PacketList *parse_queue;
>       struct PacketList *parse_queue_end;
> +    /**
> +     * The generic code uses this as a temporary packet
> +     * to parse packets; it may also be used for other means
> +     * for short periods that are guaranteed not to overlap
> +     * with calls to av_read_frame() (or ff_read_packet())
> +     * or with each other.
> +     * Every user has to ensure that this packet is blank
> +     * after using it.
> +     */
> +    AVPacket *parse_pkt;
>   
>       /**
>        * Used to hold temporary packets.
> @@ -190,7 +199,6 @@ struct AVStreamInternal {
>        * supported) */
>       struct {
>           AVBSFContext *bsf;
> -        AVPacket     *pkt;
>           int inited;
>       } extract_extradata;
>   
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 0c167d0cb9..3542e40afd 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3498,13 +3498,9 @@ static int extract_extradata_init(AVStream *st)
>       if (!ret)
>           goto finish;
>   
> -    sti->extract_extradata.pkt = av_packet_alloc();
> -    if (!sti->extract_extradata.pkt)
> -        return AVERROR(ENOMEM);
> -
>       ret = av_bsf_alloc(f, &sti->extract_extradata.bsf);
>       if (ret < 0)
> -        goto fail;
> +        return ret;
>   
>       ret = avcodec_parameters_copy(sti->extract_extradata.bsf->par_in,
>                                     st->codecpar);
> @@ -3523,14 +3519,12 @@ finish:
>       return 0;
>   fail:
>       av_bsf_free(&sti->extract_extradata.bsf);
> -    av_packet_free(&sti->extract_extradata.pkt);
>       return ret;
>   }
>   
> -static int extract_extradata(AVStream *st, const AVPacket *pkt)
> +static int extract_extradata(AVStream *st, AVPacket *tmp, const AVPacket *pkt)

Make the caller pass the AVFormatContext pointer and set pkt_ref to 
ic->internal->parse_pkt instead of sti->extract_extradata.pkt. It's more 
in line with how we use internal fields as temporary storage in most 
functions, and will reduce the size of this patch.

>   {
>       AVStreamInternal *sti = st->internal;
> -    AVPacket *pkt_ref;
>       int ret;
>   
>       if (!sti->extract_extradata.inited) {
> @@ -3542,27 +3536,26 @@ static int extract_extradata(AVStream *st, const AVPacket *pkt)
>       if (sti->extract_extradata.inited && !sti->extract_extradata.bsf)
>           return 0;
>   
> -    pkt_ref = sti->extract_extradata.pkt;
> -    ret = av_packet_ref(pkt_ref, pkt);
> +    ret = av_packet_ref(tmp, pkt);
>       if (ret < 0)
>           return ret;
>   
> -    ret = av_bsf_send_packet(sti->extract_extradata.bsf, pkt_ref);
> +    ret = av_bsf_send_packet(sti->extract_extradata.bsf, tmp);
>       if (ret < 0) {
> -        av_packet_unref(pkt_ref);
> +        av_packet_unref(tmp);
>           return ret;
>       }
>   
>       while (ret >= 0 && !sti->avctx->extradata) {
> -        ret = av_bsf_receive_packet(sti->extract_extradata.bsf, pkt_ref);
> +        ret = av_bsf_receive_packet(sti->extract_extradata.bsf, tmp);
>           if (ret < 0) {
>               if (ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
>                   return ret;
>               continue;
>           }
>   
> -        for (int i = 0; i < pkt_ref->side_data_elems; i++) {
> -            AVPacketSideData *side_data = &pkt_ref->side_data[i];
> +        for (int i = 0; i < tmp->side_data_elems; i++) {
> +            AVPacketSideData *side_data = &tmp->side_data[i];
>               if (side_data->type == AV_PKT_DATA_NEW_EXTRADATA) {
>                   sti->avctx->extradata      = side_data->data;
>                   sti->avctx->extradata_size = side_data->size;
> @@ -3571,7 +3564,7 @@ static int extract_extradata(AVStream *st, const AVPacket *pkt)
>                   break;
>               }
>           }
> -        av_packet_unref(pkt_ref);
> +        av_packet_unref(tmp);
>       }
>   
>       return 0;
> @@ -3923,7 +3916,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>                   st->internal->info->frame_delay_evidence = 1;
>           }
>           if (!st->internal->avctx->extradata) {
> -            ret = extract_extradata(st, pkt);
> +            ret = extract_extradata(st, ic->internal->parse_pkt, pkt);
>               if (ret < 0)
>                   goto unref_then_goto_end;
>           }
> @@ -4189,7 +4182,6 @@ find_stream_info_err:
>           avcodec_close(ic->streams[i]->internal->avctx);
>           av_freep(&ic->streams[i]->internal->info);
>           av_bsf_free(&ic->streams[i]->internal->extract_extradata.bsf);
> -        av_packet_free(&ic->streams[i]->internal->extract_extradata.pkt);
>       }
>       if (ic->pb)
>           av_log(ic, AV_LOG_DEBUG, "After avformat_find_stream_info() pos: %"PRId64" bytes read:%"PRId64" seeks:%d frames:%d\n",
> @@ -4391,7 +4383,6 @@ static void free_stream(AVStream **pst)
>           av_freep(&st->internal->probe_data.buf);
>   
>           av_bsf_free(&st->internal->extract_extradata.bsf);
> -        av_packet_free(&st->internal->extract_extradata.pkt);
>   
>           if (st->internal->info)
>               av_freep(&st->internal->info->duration_error);
>
diff mbox series

Patch

diff --git a/libavformat/internal.h b/libavformat/internal.h
index 3c6b2921c1..a810d51bba 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -90,9 +90,18 @@  struct AVFormatInternal {
     /**
      * Packets split by the parser get queued here.
      */
-    AVPacket *parse_pkt;
     struct PacketList *parse_queue;
     struct PacketList *parse_queue_end;
+    /**
+     * The generic code uses this as a temporary packet
+     * to parse packets; it may also be used for other means
+     * for short periods that are guaranteed not to overlap
+     * with calls to av_read_frame() (or ff_read_packet())
+     * or with each other.
+     * Every user has to ensure that this packet is blank
+     * after using it.
+     */
+    AVPacket *parse_pkt;
 
     /**
      * Used to hold temporary packets.
@@ -190,7 +199,6 @@  struct AVStreamInternal {
      * supported) */
     struct {
         AVBSFContext *bsf;
-        AVPacket     *pkt;
         int inited;
     } extract_extradata;
 
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 0c167d0cb9..3542e40afd 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3498,13 +3498,9 @@  static int extract_extradata_init(AVStream *st)
     if (!ret)
         goto finish;
 
-    sti->extract_extradata.pkt = av_packet_alloc();
-    if (!sti->extract_extradata.pkt)
-        return AVERROR(ENOMEM);
-
     ret = av_bsf_alloc(f, &sti->extract_extradata.bsf);
     if (ret < 0)
-        goto fail;
+        return ret;
 
     ret = avcodec_parameters_copy(sti->extract_extradata.bsf->par_in,
                                   st->codecpar);
@@ -3523,14 +3519,12 @@  finish:
     return 0;
 fail:
     av_bsf_free(&sti->extract_extradata.bsf);
-    av_packet_free(&sti->extract_extradata.pkt);
     return ret;
 }
 
-static int extract_extradata(AVStream *st, const AVPacket *pkt)
+static int extract_extradata(AVStream *st, AVPacket *tmp, const AVPacket *pkt)
 {
     AVStreamInternal *sti = st->internal;
-    AVPacket *pkt_ref;
     int ret;
 
     if (!sti->extract_extradata.inited) {
@@ -3542,27 +3536,26 @@  static int extract_extradata(AVStream *st, const AVPacket *pkt)
     if (sti->extract_extradata.inited && !sti->extract_extradata.bsf)
         return 0;
 
-    pkt_ref = sti->extract_extradata.pkt;
-    ret = av_packet_ref(pkt_ref, pkt);
+    ret = av_packet_ref(tmp, pkt);
     if (ret < 0)
         return ret;
 
-    ret = av_bsf_send_packet(sti->extract_extradata.bsf, pkt_ref);
+    ret = av_bsf_send_packet(sti->extract_extradata.bsf, tmp);
     if (ret < 0) {
-        av_packet_unref(pkt_ref);
+        av_packet_unref(tmp);
         return ret;
     }
 
     while (ret >= 0 && !sti->avctx->extradata) {
-        ret = av_bsf_receive_packet(sti->extract_extradata.bsf, pkt_ref);
+        ret = av_bsf_receive_packet(sti->extract_extradata.bsf, tmp);
         if (ret < 0) {
             if (ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
                 return ret;
             continue;
         }
 
-        for (int i = 0; i < pkt_ref->side_data_elems; i++) {
-            AVPacketSideData *side_data = &pkt_ref->side_data[i];
+        for (int i = 0; i < tmp->side_data_elems; i++) {
+            AVPacketSideData *side_data = &tmp->side_data[i];
             if (side_data->type == AV_PKT_DATA_NEW_EXTRADATA) {
                 sti->avctx->extradata      = side_data->data;
                 sti->avctx->extradata_size = side_data->size;
@@ -3571,7 +3564,7 @@  static int extract_extradata(AVStream *st, const AVPacket *pkt)
                 break;
             }
         }
-        av_packet_unref(pkt_ref);
+        av_packet_unref(tmp);
     }
 
     return 0;
@@ -3923,7 +3916,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
                 st->internal->info->frame_delay_evidence = 1;
         }
         if (!st->internal->avctx->extradata) {
-            ret = extract_extradata(st, pkt);
+            ret = extract_extradata(st, ic->internal->parse_pkt, pkt);
             if (ret < 0)
                 goto unref_then_goto_end;
         }
@@ -4189,7 +4182,6 @@  find_stream_info_err:
         avcodec_close(ic->streams[i]->internal->avctx);
         av_freep(&ic->streams[i]->internal->info);
         av_bsf_free(&ic->streams[i]->internal->extract_extradata.bsf);
-        av_packet_free(&ic->streams[i]->internal->extract_extradata.pkt);
     }
     if (ic->pb)
         av_log(ic, AV_LOG_DEBUG, "After avformat_find_stream_info() pos: %"PRId64" bytes read:%"PRId64" seeks:%d frames:%d\n",
@@ -4391,7 +4383,6 @@  static void free_stream(AVStream **pst)
         av_freep(&st->internal->probe_data.buf);
 
         av_bsf_free(&st->internal->extract_extradata.bsf);
-        av_packet_free(&st->internal->extract_extradata.pkt);
 
         if (st->internal->info)
             av_freep(&st->internal->info->duration_error);