[FFmpeg-devel,11/11] avformat/internal: Remove packet for extract_extradata

Submitted by Andreas Rheinhardt on Aug. 16, 2019, 3:05 a.m.

Details

Message ID 20190816030531.4775-11-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt Aug. 16, 2019, 3:05 a.m.
The effective lifetime of the packet does not extend beyond the
extract_extradata in libavformat/utils.c, so the packet can simply be
put on the stack there. This allows to remove the allocation and the
corresponding frees.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/internal.h |  1 -
 libavformat/utils.c    | 10 +---------
 2 files changed, 1 insertion(+), 10 deletions(-)

Comments

Hendrik Leppkes Aug. 16, 2019, 9:04 a.m.
On Fri, Aug 16, 2019 at 5:22 AM Andreas Rheinhardt
<andreas.rheinhardt@gmail.com> wrote:
>
> The effective lifetime of the packet does not extend beyond the
> extract_extradata in libavformat/utils.c, so the packet can simply be
> put on the stack there. This allows to remove the allocation and the
> corresponding frees.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/internal.h |  1 -
>  libavformat/utils.c    | 10 +---------
>  2 files changed, 1 insertion(+), 10 deletions(-)
>

AVPacket is defined in avcodec, as such using it outside of avcodec on
the stack feels rather iffy, as we try to get users to stop doing that
and unlock the size of AVPacket from the ABI.

- Hendrik
Andreas Rheinhardt Aug. 17, 2019, 12:33 a.m.
Hendrik Leppkes:
> On Fri, Aug 16, 2019 at 5:22 AM Andreas Rheinhardt
> <andreas.rheinhardt@gmail.com> wrote:
>>
>> The effective lifetime of the packet does not extend beyond the
>> extract_extradata in libavformat/utils.c, so the packet can simply be
>> put on the stack there. This allows to remove the allocation and the
>> corresponding frees.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavformat/internal.h |  1 -
>>  libavformat/utils.c    | 10 +---------
>>  2 files changed, 1 insertion(+), 10 deletions(-)
>>
> 
> AVPacket is defined in avcodec, as such using it outside of avcodec on
> the stack feels rather iffy, as we try to get users to stop doing that
> and unlock the size of AVPacket from the ABI.
> 
> - Hendrik

Ok, if it is the consensus that sizeof(AVPacket) should eventually not
be public API any more, then this patch makes no sense of course. So
I'll drop it.

- Andreas

Patch hide | download patch | download mbox

diff --git a/libavformat/internal.h b/libavformat/internal.h
index d6a039c497..2574d2da0c 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -182,7 +182,6 @@  struct AVStreamInternal {
      * supported) */
     struct {
         AVBSFContext *bsf;
-        AVPacket     *pkt;
         int inited;
     } extract_extradata;
 
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 64516a6a81..d3cd4b4167 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3491,10 +3491,6 @@  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;
@@ -3516,14 +3512,13 @@  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, AVPacket *pkt)
 {
     AVStreamInternal *sti = st->internal;
-    AVPacket *pkt_ref;
+    AVPacket pkt1, *pkt_ref = &pkt1;
     int ret;
 
     if (!sti->extract_extradata.inited) {
@@ -3535,7 +3530,6 @@  static int extract_extradata(AVStream *st, 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);
     if (ret < 0)
         return ret;
@@ -4171,7 +4165,6 @@  find_stream_info_err:
         avcodec_close(ic->streams[i]->internal->avctx);
         av_freep(&ic->streams[i]->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",
@@ -4369,7 +4362,6 @@  static void free_stream(AVStream **pst)
         }
         av_freep(&st->internal->priv_pts);
         av_bsf_free(&st->internal->extract_extradata.bsf);
-        av_packet_free(&st->internal->extract_extradata.pkt);
     }
     av_freep(&st->internal);