diff mbox series

[FFmpeg-devel,27/31] fftools/ffmpeg_dec: stop passing InputStream to dec_open()

Message ID 20240124081702.4759-27-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/31] fftools/ffmpeg_dec: split Decoder into a private and public part | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Jan. 24, 2024, 8:16 a.m. UTC 
---
 fftools/ffmpeg.h       |  3 ++-
 fftools/ffmpeg_dec.c   | 50 +++++++++++++++++++++++++++---------------
 fftools/ffmpeg_demux.c |  4 +++-
 3 files changed, 37 insertions(+), 20 deletions(-)

Comments

Michael Niedermayer Jan. 25, 2024, 1:19 a.m. UTC | #1 
On Wed, Jan 24, 2024 at 09:16:57AM +0100, Anton Khirnov wrote:
> ---
>  fftools/ffmpeg.h       |  3 ++-
>  fftools/ffmpeg_dec.c   | 50 +++++++++++++++++++++++++++---------------
>  fftools/ffmpeg_demux.c |  4 +++-
>  3 files changed, 37 insertions(+), 20 deletions(-)

This causes segfaults:
(ill mail you the sample)

  Metadata:
    creation_time   : 1999-10-07T10:13:38.000000Z
  Duration: 00:00:17.54, start: 0.000000, bitrate: 1980 kb/s
  Stream #0:0[0x1](eng): Video: qtrle (rle  / 0x20656C72), none, 310x280, 438202 kb/s, 25 fps, 25 tbr, 600 tbn (default)
      Metadata:
        creation_time   : 1999-10-07T10:13:38.000000Z
        handler_name    : Apple Video Media Handler
        vendor_id       : appl
        encoder         : Animation
[qtrle @ 0x16a2bf40] Unsupported colorspace: 108 bits/sample?
==24457==    at 0x13E0719: VALGRIND_PRINTF_BACKTRACE (valgrind.h:6303)
==24457==    by 0x13E1318: av_log_default_callback (log.c:404)
==24457==    by 0x13E15BF: av_vlog (log.c:439)
==24457==    by 0x13E141E: av_log (log.c:418)
==24457==    by 0xC7C031: qtrle_decode_init (qtrle.c:434)
==24457==    by 0x88AF88: avcodec_open2 (avcodec.c:337)
==24457==    by 0x2448EE: dec_open (ffmpeg_dec.c:1024)
==24457==    by 0x2486D7: ist_use (ffmpeg_demux.c:938)
==24457==    by 0x248835: ist_filter_add (ffmpeg_demux.c:973)
==24457==    by 0x253BC5: ifilter_bind_ist (ffmpeg_filter.c:676)
==24457==    by 0x254C21: init_simple_filtergraph (ffmpeg_filter.c:1072)
==24457==    by 0x26ADAB: ost_add (ffmpeg_mux_init.c:1444)
==24457==    by 0x26B491: map_auto_video (ffmpeg_mux_init.c:1550)
==24457==    by 0x26C3A1: create_streams (ffmpeg_mux_init.c:1858)
==24457==    by 0x270A6C: of_open (ffmpeg_mux_init.c:3091)
==24457==    by 0x274D48: open_files (ffmpeg_opt.c:1333)
==24457==    by 0x274F74: ffmpeg_parse_options (ffmpeg_opt.c:1387)
==24457==    by 0x289391: main (ffmpeg.c:1032)
[vist#0:0/qtrle @ 0x16a25e80] [dec:qtrle @ 0x16a2b9c0] Error while opening decoder: Invalid data found when processing input
==24457==    at 0x13E0719: VALGRIND_PRINTF_BACKTRACE (valgrind.h:6303)
==24457==    by 0x13E1318: av_log_default_callback (log.c:404)
==24457==    by 0x13E15BF: av_vlog (log.c:439)
==24457==    by 0x13E141E: av_log (log.c:418)
==24457==    by 0x24494E: dec_open (ffmpeg_dec.c:1025)
==24457==    by 0x2486D7: ist_use (ffmpeg_demux.c:938)
==24457==    by 0x248835: ist_filter_add (ffmpeg_demux.c:973)
==24457==    by 0x253BC5: ifilter_bind_ist (ffmpeg_filter.c:676)
==24457==    by 0x254C21: init_simple_filtergraph (ffmpeg_filter.c:1072)
==24457==    by 0x26ADAB: ost_add (ffmpeg_mux_init.c:1444)
==24457==    by 0x26B491: map_auto_video (ffmpeg_mux_init.c:1550)
==24457==    by 0x26C3A1: create_streams (ffmpeg_mux_init.c:1858)
==24457==    by 0x270A6C: of_open (ffmpeg_mux_init.c:3091)
==24457==    by 0x274D48: open_files (ffmpeg_opt.c:1333)
==24457==    by 0x274F74: ffmpeg_parse_options (ffmpeg_opt.c:1387)
==24457==    by 0x289391: main (ffmpeg.c:1032)
==24457==
==24457== Process terminating with default action of signal 11 (SIGSEGV)
==24457==  Bad permissions for mapped region at address 0x1B68D28
==24457==    at 0x13E45C9: av_freep (mem.c:250)
==24457==    by 0x241E09: dec_free (ffmpeg_dec.c:108)
==24457==    by 0x2449D4: dec_open (ffmpeg_dec.c:1041)
==24457==    by 0x2486D7: ist_use (ffmpeg_demux.c:938)
==24457==    by 0x248835: ist_filter_add (ffmpeg_demux.c:973)
==24457==    by 0x253BC5: ifilter_bind_ist (ffmpeg_filter.c:676)
==24457==    by 0x254C21: init_simple_filtergraph (ffmpeg_filter.c:1072)
==24457==    by 0x26ADAB: ost_add (ffmpeg_mux_init.c:1444)
==24457==    by 0x26B491: map_auto_video (ffmpeg_mux_init.c:1550)
==24457==    by 0x26C3A1: create_streams (ffmpeg_mux_init.c:1858)
==24457==    by 0x270A6C: of_open (ffmpeg_mux_init.c:3091)
==24457==    by 0x274D48: open_files (ffmpeg_opt.c:1333)
==24457==    by 0x274F74: ffmpeg_parse_options (ffmpeg_opt.c:1387)
==24457==    by 0x289391: main (ffmpeg.c:1032)


[...]
diff mbox series

Patch

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 02c614d0ff..c8cba72006 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -296,6 +296,7 @@  typedef struct DecoderOpts {
     int                         flags;
 
     char                       *name;
+    void                       *log_parent;
 
     const AVCodec              *codec;
     const AVCodecParameters    *par;
@@ -756,7 +757,7 @@  AVBufferRef *hw_device_for_filter(void);
  * @param dec_opts Dictionary filled with decoder options. Its ownership
  *                 is transferred to the decoder.
  */
-int dec_open(InputStream *ist, Scheduler *sch, unsigned sch_idx,
+int dec_open(Decoder **pdec, Scheduler *sch, unsigned sch_idx,
              AVDictionary **dec_opts, const DecoderOpts *o);
 void dec_free(Decoder **pdec);
 
diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index 98cde0a040..3610256f31 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -110,7 +110,7 @@  void dec_free(Decoder **pdec)
     av_freep(pdec);
 }
 
-static int dec_alloc(Decoder **pdec)
+static int dec_alloc(DecoderPriv **pdec)
 {
     DecoderPriv *dp;
 
@@ -133,7 +133,7 @@  static int dec_alloc(Decoder **pdec)
     dp->last_frame_tb                = (AVRational){ 1, 1 };
     dp->hwaccel_pix_fmt              = AV_PIX_FMT_NONE;
 
-    *pdec = &dp->dec;
+    *pdec = dp;
 
     return 0;
 fail:
@@ -940,24 +940,25 @@  static const AVClass dec_class = {
     .item_name                 = dec_item_name,
 };
 
-int dec_open(InputStream *ist, Scheduler *sch, unsigned sch_idx,
+int dec_open(Decoder **pdec, Scheduler *sch, unsigned sch_idx,
              AVDictionary **dec_opts, const DecoderOpts *o)
 {
     DecoderPriv *dp;
     const AVCodec *codec = o->codec;
     int ret;
 
-    ret = dec_alloc(&ist->decoder);
+    *pdec = NULL;
+
+    ret = dec_alloc(&dp);
     if (ret < 0)
         return ret;
-    dp = dp_from_dec(ist->decoder);
 
     dp->sch     = sch;
     dp->sch_idx = sch_idx;
 
     dp->flags      = o->flags;
     dp->dec.class  = &dec_class;
-    dp->log_parent = ist;
+    dp->log_parent = o->log_parent;
 
     dp->framerate_in            = o->framerate;
 
@@ -968,31 +969,39 @@  int dec_open(InputStream *ist, Scheduler *sch, unsigned sch_idx,
     snprintf(dp->log_name, sizeof(dp->log_name), "dec:%s", codec->name);
 
     dp->parent_name = av_strdup(o->name ? o->name : "");
-    if (!dp->parent_name)
-        return AVERROR(ENOMEM);
+    if (!dp->parent_name) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
 
     if (codec->type == AVMEDIA_TYPE_SUBTITLE &&
         (dp->flags & DECODER_FLAG_FIX_SUB_DURATION)) {
         for (int i = 0; i < FF_ARRAY_ELEMS(dp->sub_prev); i++) {
             dp->sub_prev[i] = av_frame_alloc();
-            if (!dp->sub_prev[i])
-                return AVERROR(ENOMEM);
+            if (!dp->sub_prev[i]) {
+                ret = AVERROR(ENOMEM);
+                goto fail;
+            }
         }
         dp->sub_heartbeat = av_frame_alloc();
-        if (!dp->sub_heartbeat)
-            return AVERROR(ENOMEM);
+        if (!dp->sub_heartbeat) {
+            ret = AVERROR(ENOMEM);
+            goto fail;
+        }
     }
 
     dp->sar_override = o->par->sample_aspect_ratio;
 
     dp->dec_ctx = avcodec_alloc_context3(codec);
-    if (!dp->dec_ctx)
-        return AVERROR(ENOMEM);
+    if (!dp->dec_ctx) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
 
     ret = avcodec_parameters_to_context(dp->dec_ctx, o->par);
     if (ret < 0) {
         av_log(dp, AV_LOG_ERROR, "Error initializing the decoder context.\n");
-        return ret;
+        goto fail;
     }
 
     dp->dec_ctx->opaque                = dp;
@@ -1009,23 +1018,28 @@  int dec_open(InputStream *ist, Scheduler *sch, unsigned sch_idx,
         av_log(dp, AV_LOG_ERROR,
                "Hardware device setup failed for decoder: %s\n",
                av_err2str(ret));
-        return ret;
+        goto fail;
     }
 
     if ((ret = avcodec_open2(dp->dec_ctx, codec, dec_opts)) < 0) {
         av_log(dp, AV_LOG_ERROR, "Error while opening decoder: %s\n",
                av_err2str(ret));
-        return ret;
+        goto fail;
     }
 
     ret = check_avoptions(*dec_opts);
     if (ret < 0)
-        return ret;
+        goto fail;
 
     dp->dec.subtitle_header      = dp->dec_ctx->subtitle_header;
     dp->dec.subtitle_header_size = dp->dec_ctx->subtitle_header_size;
 
+    *pdec = &dp->dec;
+
     return 0;
+fail:
+    dec_free((Decoder**)dp);
+    return ret;
 }
 
 int dec_add_filter(Decoder *dec, InputFilter *ifilter)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 6216b7c684..5178bcbce4 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -933,7 +933,9 @@  static int ist_use(InputStream *ist, int decoding_needed)
         ds->dec_opts.codec = ist->dec;
         ds->dec_opts.par   = ist->par;
 
-        ret = dec_open(ist, d->sch, ds->sch_idx_dec,
+        ds->dec_opts.log_parent = ist;
+
+        ret = dec_open(&ist->decoder, d->sch, ds->sch_idx_dec,
                        &ist->decoder_opts, &ds->dec_opts);
         if (ret < 0)
             return ret;