[FFmpeg-devel] [PATCH] mjpegdec: ensure SOF before SOS/EOI

[Reimar Döffinger]

On Wed, Jul 01, 2009 at 10:15:22PM +0200, Reimar D?ffinger wrote:
> this fixes issue1240, mjpeg/smclockmjpeg.avi.1.0
> The problem is, it reaches EOI before SOF, thus get_buffer was
> never called and the picture it returns has invalid values for
> data[] (NULL in this case, but it could be values from a previous
> frame which still would be wrong).
> This could also happen, if avcodec_check_dimensions failed, which
> might make this even more of an issue.
> Attached patch fixes it and also protects the decode_sos part so
> it will only be called with a valid picture.

Any objections?
As a second step I suggest extending the code as in attached patch.
This allows to decode JPEGs that have a misplaced EOI somewhere too
early, decodes images that lack an EOI marker and most importantly
makes the decoder fail and return -1 when the parsed data is complete
gibberish (no e.g. no SOF at all).
AFAICT currently when you pass random data to the MJPEG decoder it will
basically say "hey, everything's fine, I just don't have any decoded
data yet" which IMO is inappropriate.
-------------- next part --------------
Index: libavcodec/mjpegdec.c
===================================================================
--- libavcodec/mjpegdec.c	(revision 19334)
+++ libavcodec/mjpegdec.c	(working copy)
@@ -338,6 +338,7 @@
     }
     s->picture.pict_type= FF_I_TYPE;
     s->picture.key_frame= 1;
+    s->got_picture = 1;
 
     for(i=0; i<3; i++){
         s->linesize[i]= s->picture.linesize[i] << s->interlaced;
@@ -1265,6 +1266,7 @@
     int start_code;
     AVFrame *picture = data;
 
+    s->got_picture = 0; // picture from previous image can not be reused
     buf_ptr = buf;
     buf_end = buf + buf_size;
     while (buf_ptr < buf_end) {
@@ -1426,6 +1428,10 @@
                     if ((s->buggy_avid && !s->interlaced) || s->restart_interval)
                         break;
 eoi_parser:
+                    if (!s->got_picture) {
+                        av_log(avctx, AV_LOG_WARNING, "Found EOI before any SOF\n");
+                        break;
+                    }
                     {
                         if (s->interlaced) {
                             s->bottom_field ^= 1;
@@ -1450,6 +1456,8 @@
                     }
                     break;
                 case SOS:
+                    if (!s->got_picture)
+                        return -1;
                     ff_mjpeg_decode_sos(s);
                     /* buggy avid puts EOI every 10-20th frame */
                     /* if restart period is over process EOI */
@@ -1485,6 +1493,12 @@
             }
         }
     }
+    if (s->got_picture) {
+        av_log(avctx, AV_LOG_WARNING, "EOI missing, emulating\n");
+        goto eoi_parser;
+    }
+    av_log(avctx, AV_LOG_FATAL, "No JPEG data found in image\n");
+    return -1;
 the_end:
     av_log(avctx, AV_LOG_DEBUG, "mjpeg decode frame unused %td bytes\n", buf_end - buf_ptr);
 //    return buf_end - buf_ptr;
Index: libavcodec/mjpegdec.h
===================================================================
--- libavcodec/mjpegdec.h	(revision 19334)
+++ libavcodec/mjpegdec.h	(working copy)
@@ -81,6 +81,7 @@
     int quant_index[4];   /* quant table index for each component */
     int last_dc[MAX_COMPONENTS]; /* last DEQUANTIZED dc (XXX: am I right to do that ?) */
     AVFrame picture; /* picture structure */
+    int got_picture;                                ///< we found a SOF and picture is valid, too.
     int linesize[MAX_COMPONENTS];                   ///< linesize << interlaced
     int8_t *qscale_table;
     DECLARE_ALIGNED_16(DCTELEM, block[64]);