ext/ffmpeg/: Latest ffmpeg revision's avcodec_close frees more; use safer coding...

Original commit message from CVS:
* ext/ffmpeg/gstffmpegcfg.c: (gst_ffmpeg_cfg_fill_context):
* ext/ffmpeg/gstffmpegenc.c: (gst_ffmpegenc_getcaps):
Latest ffmpeg revision's avcodec_close frees more; use safer coding to
prevent double free and other related segfaults.

diff --git a/ChangeLog b/ChangeLog
index a48a126..5786bcc 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2008-08-01  Mark Nauwelaerts  <mark.nauwelaerts@collabora.co.uk>
+
+       * ext/ffmpeg/gstffmpegcfg.c: (gst_ffmpeg_cfg_fill_context):
+       * ext/ffmpeg/gstffmpegenc.c: (gst_ffmpegenc_getcaps):
+       Latest ffmpeg revision's avcodec_close frees more; use safer coding to
+       prevent double free and other related segfaults.
+
 2008-07-28  Mark Nauwelaerts  <mark.nauwelaerts@collabora.co.uk>
 
        * ext/ffmpeg/gstffmpegcodecmap.c: (gst_ffmpeg_caps_to_pixfmt):

diff --git a/common b/common
index e798798..d70ca17 160000 (submodule)
--- a/common
+++ b/common
@@ -1 +1 @@
-Subproject commit e79879859bc866545379eb77e1378a906dc30ebf
+Subproject commit d70ca17ae6fbe6020996e4567275d5e14972ed45

diff --git a/ext/ffmpeg/gstffmpegcfg.c b/ext/ffmpeg/gstffmpegcfg.c
index 32399c2..55076c7 100644 (file)
--- a/ext/ffmpeg/gstffmpegcfg.c
+++ b/ext/ffmpeg/gstffmpegcfg.c
@@ -1013,9 +1013,17 @@ gst_ffmpeg_cfg_fill_context (GstFFMpegEnc * ffmpegenc, AVCodecContext * context)
     context_offset = qdata->offset - CONTEXT_CONFIG_OFFSET;
     if (gst_ffmpeg_cfg_codec_has_pspec (klass->in_plugin->id, pspec)
         && context_offset >= 0) {
-      /* memcpy a bit heavy for a small copy, but hardly part of 'inner loop' */
-      memcpy (G_STRUCT_MEMBER_P (context, context_offset),
-          G_STRUCT_MEMBER_P (ffmpegenc, qdata->offset), qdata->size);
+      if (G_PARAM_SPEC_VALUE_TYPE (pspec) == G_TYPE_STRING) {
+        /* make a copy for ffmpeg, it will likely free only some,
+         * but in any case safer than a potential double free */
+        G_STRUCT_MEMBER (gchar *, context, context_offset) =
+            g_strdup (G_STRUCT_MEMBER (gchar *, ffmpegenc, qdata->offset));
+      } else {
+        /* memcpy a bit heavy for a small copy,
+         * but hardly part of 'inner loop' */
+        memcpy (G_STRUCT_MEMBER_P (context, context_offset),
+            G_STRUCT_MEMBER_P (ffmpegenc, qdata->offset), qdata->size);
+      }
     }
     list = list->next;
   }

diff --git a/ext/ffmpeg/gstffmpegenc.c b/ext/ffmpeg/gstffmpegenc.c
index 00c73e9..bc51699 100644 (file)
--- a/ext/ffmpeg/gstffmpegenc.c
+++ b/ext/ffmpeg/gstffmpegenc.c
@@ -283,7 +283,7 @@ gst_ffmpegenc_getcaps (GstPad * pad)
   GstFFMpegEnc *ffmpegenc = (GstFFMpegEnc *) GST_PAD_PARENT (pad);
   GstFFMpegEncClass *oclass =
       (GstFFMpegEncClass *) G_OBJECT_GET_CLASS (ffmpegenc);
-  AVCodecContext *ctx;
+  AVCodecContext *ctx = NULL;
   enum PixelFormat pixfmt;
   GstCaps *caps = NULL;
 
@@ -306,21 +306,6 @@ gst_ffmpegenc_getcaps (GstPad * pad)
   }
 
   /* create cache etc. */
-  ctx = avcodec_alloc_context ();
-  if (!ctx) {
-    caps = gst_caps_copy (gst_pad_get_pad_template_caps (pad));
-    GST_DEBUG_OBJECT (ffmpegenc, "no context, return template caps %"GST_PTR_FORMAT, caps);
-    return caps;
-  }
-
-  /* set some default properties */
-  ctx->width = DEFAULT_WIDTH;
-  ctx->height = DEFAULT_HEIGHT;
-  ctx->time_base.num = DEFAULT_FRAME_RATE_BASE;
-  ctx->time_base.den = 25 * DEFAULT_FRAME_RATE_BASE;
-  ctx->bit_rate = DEFAULT_VIDEO_BITRATE;
-  /* makes it silent */
-  ctx->strict_std_compliance = -1;
 
   /* shut up the logging while we autoprobe; we don't want warnings and
    * errors about unsupported formats */
@@ -335,6 +320,23 @@ gst_ffmpegenc_getcaps (GstPad * pad)
   for (pixfmt = 0; pixfmt < PIX_FMT_NB; pixfmt++) {
     GstCaps *tmpcaps;
 
+    /* need to start with a fresh codec_context each time around, since
+     * codec_close may have released stuff causing the next pass to segfault */
+    ctx = avcodec_alloc_context ();
+    if (!ctx) {
+      GST_DEBUG_OBJECT (ffmpegenc, "no context");
+      break;
+    }
+
+    /* set some default properties */
+    ctx->width = DEFAULT_WIDTH;
+    ctx->height = DEFAULT_HEIGHT;
+    ctx->time_base.num = DEFAULT_FRAME_RATE_BASE;
+    ctx->time_base.den = 25 * DEFAULT_FRAME_RATE_BASE;
+    ctx->bit_rate = DEFAULT_VIDEO_BITRATE;
+    /* makes it silent */
+    ctx->strict_std_compliance = -1;
+
     ctx->pix_fmt = pixfmt;
     if (gst_ffmpeg_avcodec_open (ctx, oclass->in_plugin) >= 0 &&
         ctx->pix_fmt == pixfmt) {
@@ -353,8 +355,8 @@ gst_ffmpegenc_getcaps (GstPad * pad)
     }
     if (ctx->priv_data)
       gst_ffmpeg_avcodec_close (ctx);
+    av_free (ctx);
   }
-  av_free (ctx);
 #ifndef GST_DISABLE_GST_DEBUG
   _shut_up_I_am_probing = FALSE;
 #endif
@@ -362,7 +364,8 @@ gst_ffmpegenc_getcaps (GstPad * pad)
   /* make sure we have something */
   if (!caps) {
     caps = gst_caps_copy (gst_pad_get_pad_template_caps (pad));
-    GST_DEBUG_OBJECT (ffmpegenc, "probing gave nothing, return template %"GST_PTR_FORMAT, caps);
+    GST_DEBUG_OBJECT (ffmpegenc, "probing gave nothing, "
+        "return template %" GST_PTR_FORMAT, caps);
     return caps;
   }