[FFmpeg-devel] [PATCH] avcodec/vc1dec: Re-order init to avoid initting hwaccel too early

[Philip Langdale]

At least for vdpau, the hwaccel init code tries to check the video
profile and ensure that there is a matching vdpau profile available.

If it can't find a match, it will fail to initialise.

In the case of wmv3/vc1, I observed initialisation to fail all the
time. It turns out that this is due to the hwaccel being initialised
very early in the codec init, before the profile has been extracted
and set.

Conceptually, it's a simple fix to reorder the init code, but it gets
messy really fast because ff_get_format(), which is what implicitly
trigger hwaccel init, is called multiple times through various shared
init calls from h263, etc. It's incredibly hard to prove to my own
satisfaction that it's safe to move the vc1 specific init code
ahead of this generic code, but all the vc1 fate tests pass, and I've
visually inspected a couple of samples and things seem correct.

Signed-off-by: Philip Langdale <philipl at overt.org>
---
 libavcodec/vc1dec.c | 52 ++++++++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index abaa0b4..e8e79af 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -428,30 +428,10 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
 
     if (!avctx->extradata_size || !avctx->extradata)
         return -1;
-    if (!CONFIG_GRAY || !(avctx->flags & AV_CODEC_FLAG_GRAY))
-        avctx->pix_fmt = ff_get_format(avctx, avctx->codec->pix_fmts);
-    else {
-        avctx->pix_fmt = AV_PIX_FMT_GRAY8;
-        if (avctx->color_range == AVCOL_RANGE_UNSPECIFIED)
-            avctx->color_range = AVCOL_RANGE_MPEG;
-    }
     v->s.avctx = avctx;
 
     if ((ret = ff_vc1_init_common(v)) < 0)
         return ret;
-    // ensure static VLC tables are initialized
-    if ((ret = ff_msmpeg4_decode_init(avctx)) < 0)
-        return ret;
-    if ((ret = ff_vc1_decode_init_alloc_tables(v)) < 0)
-        return ret;
-    // Hack to ensure the above functions will be called
-    // again once we know all necessary settings.
-    // That this is necessary might indicate a bug.
-    ff_vc1_decode_end(avctx);
-
-    ff_blockdsp_init(&s->bdsp, avctx);
-    ff_h264chroma_init(&v->h264chroma, 8);
-    ff_qpeldsp_init(&s->qdsp);
 
     if (avctx->codec_id == AV_CODEC_ID_WMV3 || avctx->codec_id == AV_CODEC_ID_WMV3IMAGE) {
         int count = 0;
@@ -524,14 +504,38 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
         v->res_sprite = (avctx->codec_id == AV_CODEC_ID_VC1IMAGE);
     }
 
-    v->sprite_output_frame = av_frame_alloc();
-    if (!v->sprite_output_frame)
-        return AVERROR(ENOMEM);
-
     avctx->profile = v->profile;
     if (v->profile == PROFILE_ADVANCED)
         avctx->level = v->level;
 
+    if (!CONFIG_GRAY || !(avctx->flags & AV_CODEC_FLAG_GRAY))
+        avctx->pix_fmt = ff_get_format(avctx, avctx->codec->pix_fmts);
+    else {
+        avctx->pix_fmt = AV_PIX_FMT_GRAY8;
+        if (avctx->color_range == AVCOL_RANGE_UNSPECIFIED)
+            avctx->color_range = AVCOL_RANGE_MPEG;
+    }
+
+    // ensure static VLC tables are initialized
+    if ((ret = ff_msmpeg4_decode_init(avctx)) < 0)
+        return ret;
+    if ((ret = ff_vc1_decode_init_alloc_tables(v)) < 0)
+        return ret;
+    // Hack to ensure the above functions will be called
+    // again once we know all necessary settings.
+    // That this is necessary might indicate a bug.
+    ff_vc1_decode_end(avctx);
+
+    ff_blockdsp_init(&s->bdsp, avctx);
+    ff_h264chroma_init(&v->h264chroma, 8);
+    ff_qpeldsp_init(&s->qdsp);
+
+    // Must happen after calling ff_vc1_decode_end
+    // to avoid de-allocating the sprite_output_frame
+    v->sprite_output_frame = av_frame_alloc();
+    if (!v->sprite_output_frame)
+        return AVERROR(ENOMEM);
+
     avctx->has_b_frames = !!avctx->max_b_frames;
 
     if (v->color_prim == 1 || v->color_prim == 5 || v->color_prim == 6)
-- 
2.1.4