[MPlayer-dev-eng] [PATCH] -demuxer lavf and svq3 mov files

[Alan Curry]

mplayer -demuxer lavf crashes on SVQ3 mov's. This is caused by confusion over
where the extradata is supposed to be stored. vd_ffmpeg expects extradata to
be in sh_video->ImageDesc if the codec is SVQ3. That's where demux_mov puts
it. demux_lavf puts the extradata at the end of the BITMAPINFOHEADER
regardless of the codec.

svq3.c parses the extradata with some sloppy code that doesn't check for NULL
pointers or buffer sizes, causing the crash.

There are a couple of ways to get the extradata where it needs to be: fix
demux_lavf to do what demux_mov does (making a new special case for the SVQ3
codec), or fix vd_ffmpeg to look in both places (easier since it already has
a SVQ3 special case).

The sample file mentioned in the comment at the top of libavcodec/svq3.c:

  ftp://ftp.mplayerhq.hu/MPlayer/samples/V-codecs/SVQ3/Vertical400kbit.sorenson3.mov

provides a good test case for this patch. With the current codec, it crashes
mplayer -demuxer lavf. If you just apply a minimal fix to svq3.c to prevent
the NULL pointer dereference, it doesn't crash but doesn't decode correctly
either because it really needs to see the extradata. With the following patch,
it plays correctly.

The patch is in 2 parts. The first part is a bug fix that I think should be
worthwhile on its own, but I don't have any affected files (AVRn? Pegasus
MJPEG?) to test it with. Adding sizeof(BITMAPINFOHEADER) to something that's
already a BITMAPINFOHEADER * just can't be right...

The second part moves the SVQ3 case up in the switch so it can fallthrough to
the BITMAPINFOHEADER code when ImageDesc is NULL.

And - bonus! - I just noticed that vd_qtvideo already has something similar:
    if(!sh->ImageDesc) sh->ImageDesc=(sh->bih+1); // hack for SVQ3-in-AVI
which makes me even more confident that this is a good solution.

PART 1:

Index: libmpcodecs/vd_ffmpeg.c
===================================================================
--- libmpcodecs/vd_ffmpeg.c	(revision 19256)
+++ libmpcodecs/vd_ffmpeg.c	(working copy)
@@ -274,8 +274,7 @@
 	avctx->flags |= CODEC_FLAG_EXTERN_HUFF;
 	avctx->extradata_size = sh->bih->biSize-sizeof(BITMAPINFOHEADER);
 	avctx->extradata = av_mallocz(avctx->extradata_size + FF_INPUT_BUFFER_PADDING_SIZE);
-	memcpy(avctx->extradata, sh->bih+sizeof(BITMAPINFOHEADER),
-	    avctx->extradata_size);
+	memcpy(avctx->extradata, sh->bih+1, avctx->extradata_size);
 
 #if 0
 	{

PART 2:

diff -u libmpcodecs/vd_ffmpeg.c libmpcodecs/vd_ffmpeg.c
--- libmpcodecs/vd_ffmpeg.c	(working copy)
+++ libmpcodecs/vd_ffmpeg.c	(working copy)
@@ -264,6 +264,18 @@
     avctx->skip_frame = str2AVDiscard(lavc_param_skip_frame_str);
     mp_dbg(MSGT_DECVIDEO,MSGL_DBG2,"libavcodec.size: %d x %d\n",avctx->width,avctx->height);
     switch (sh->format) {
+    case mmioFOURCC('S','V','Q','3'):
+    /* SVQ3 extradata can show up as sh->ImageDesc if demux_mov is used, or
+       in the phony AVI header if demux_lavf is used. The first case is
+       handled here; the second case falls through to the next section. */
+	if (sh->ImageDesc) {
+	    avctx->extradata_size = (*(int*)sh->ImageDesc) - sizeof(int);
+	    avctx->extradata = av_mallocz(avctx->extradata_size + FF_INPUT_BUFFER_PADDING_SIZE);
+	    memcpy(avctx->extradata, ((int*)sh->ImageDesc)+1, avctx->extradata_size);
+	    break;
+	}
+	/* fallthrough */
+
     case mmioFOURCC('A','V','R','n'):
     case mmioFOURCC('M','J','P','G'):
     /* AVRn stores huffman table in AVI header */
@@ -312,14 +324,6 @@
 //        printf("%X %X %d %d\n", extrahdr[0], extrahdr[1]);
         break;
 
-    case mmioFOURCC('S','V','Q','3'):
-	if (!sh->ImageDesc)
-	    break;
-	avctx->extradata_size = (*(int*)sh->ImageDesc) - sizeof(int);
-	avctx->extradata = av_mallocz(avctx->extradata_size + FF_INPUT_BUFFER_PADDING_SIZE);
-	memcpy(avctx->extradata, ((int*)sh->ImageDesc)+1, avctx->extradata_size);
-	break;
-
     default:
 	if (!sh->bih || sh->bih->biSize <= sizeof(BITMAPINFOHEADER))
 	    break;