[FFmpeg-devel] [PATCH 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker

James Almer jamrial at gmail.com
Thu Apr 22 20:02:33 EEST 2021


With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with
the LSE marker show up after SOF but before SOS. For those, the pixel format
chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE.
This has not been an issue given both pixel formats allocate the second data
plane for the palette, but after the upcoming soname bump, GRAY8 will no longer
do that. This will result in segfauls when ff_jpegls_decode_lse() attempts to
write the palette on a buffer originally allocated as a GRAY8 one.

Work around this by calling ff_get_buffer() after the actual pixel format is
known.

Signed-off-by: James Almer <jamrial at gmail.com>
---
Now splitting the variable that signals that a frame was allocated and that
SOF data was successfully parsed.

 libavcodec/jpeglsdec.c |  6 +--
 libavcodec/mjpegbdec.c |  1 +
 libavcodec/mjpegdec.c  | 83 +++++++++++++++++++++++++-----------------
 libavcodec/mjpegdec.h  |  3 ++
 libavcodec/mxpegdec.c  |  2 +-
 5 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
index e17de09e9f..4ab0cb5bd8 100644
--- a/libavcodec/jpeglsdec.c
+++ b/libavcodec/jpeglsdec.c
@@ -108,9 +108,8 @@ int ff_jpegls_decode_lse(MJpegDecodeContext *s)
         if (s->palette_index > maxtab)
             return AVERROR_INVALIDDATA;
 
-        if ((s->avctx->pix_fmt == AV_PIX_FMT_GRAY8 || s->avctx->pix_fmt == AV_PIX_FMT_PAL8) &&
-            (s->picture_ptr->format == AV_PIX_FMT_GRAY8 || s->picture_ptr->format == AV_PIX_FMT_PAL8)) {
-            uint32_t *pal = (uint32_t *)s->picture_ptr->data[1];
+        if (s->avctx->pix_fmt == AV_PIX_FMT_GRAY8 || s->avctx->pix_fmt == AV_PIX_FMT_PAL8) {
+            uint32_t *pal = s->palette;
             int shift = 0;
 
             if (s->avctx->bits_per_raw_sample > 0 && s->avctx->bits_per_raw_sample < 8) {
@@ -118,7 +117,6 @@ int ff_jpegls_decode_lse(MJpegDecodeContext *s)
                 shift = 8 - s->avctx->bits_per_raw_sample;
             }
 
-            s->picture_ptr->format =
             s->avctx->pix_fmt = AV_PIX_FMT_PAL8;
             for (i=s->palette_index; i<=maxtab; i++) {
                 uint8_t k = i << shift;
diff --git a/libavcodec/mjpegbdec.c b/libavcodec/mjpegbdec.c
index 7666674908..890befb522 100644
--- a/libavcodec/mjpegbdec.c
+++ b/libavcodec/mjpegbdec.c
@@ -55,6 +55,7 @@ static int mjpegb_decode_frame(AVCodecContext *avctx,
 
     buf_ptr = buf;
     buf_end = buf + buf_size;
+    s->seen_sof = 0;
     s->got_picture = 0;
     s->adobe_transform = -1;
 
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 7c7cc20af8..42e6170469 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -138,6 +138,7 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx)
     s->buffer        = NULL;
     s->start_code    = -1;
     s->first_picture = 1;
+    s->seen_sof      = 0;
     s->got_picture   = 0;
     s->orig_height    = avctx->coded_height;
     avctx->chroma_sample_location = AVCHROMA_LOC_CENTER;
@@ -429,6 +430,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
         memcpy(s->h_count, h_count, sizeof(h_count));
         memcpy(s->v_count, v_count, sizeof(v_count));
         s->interlaced = 0;
+        s->seen_sof = 0;
         s->got_picture = 0;
 
         /* test interlaced mode */
@@ -681,11 +683,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
             } else if (s->nb_components != 1) {
                 av_log(s->avctx, AV_LOG_ERROR, "Unsupported number of components %d\n", s->nb_components);
                 return AVERROR_PATCHWELCOME;
-            } else if (s->palette_index && s->bits <= 8)
-                s->avctx->pix_fmt = AV_PIX_FMT_PAL8;
-            else if (s->bits <= 8)
-                s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
-            else
+            } else if (s->bits <= 8) {
+                avpriv_set_systematic_pal2(s->palette, s->avctx->pix_fmt);
+                if (s->palette_index)
+                    s->avctx->pix_fmt = AV_PIX_FMT_PAL8;
+                else
+                    s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
+            } else
                 s->avctx->pix_fmt = AV_PIX_FMT_GRAY16;
         }
 
@@ -719,26 +723,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
         if (s->avctx->skip_frame == AVDISCARD_ALL) {
             s->picture_ptr->pict_type = AV_PICTURE_TYPE_I;
             s->picture_ptr->key_frame = 1;
-            s->got_picture            = 1;
+            s->seen_sof               = 1;
             return 0;
         }
-
-        av_frame_unref(s->picture_ptr);
-        if (ff_get_buffer(s->avctx, s->picture_ptr, AV_GET_BUFFER_FLAG_REF) < 0)
-            return -1;
-        s->picture_ptr->pict_type = AV_PICTURE_TYPE_I;
-        s->picture_ptr->key_frame = 1;
-        s->got_picture            = 1;
-
-        for (i = 0; i < 4; i++)
-            s->linesize[i] = s->picture_ptr->linesize[i] << s->interlaced;
-
-        ff_dlog(s->avctx, "%d %d %d %d %d %d\n",
-                s->width, s->height, s->linesize[0], s->linesize[1],
-                s->interlaced, s->avctx->height);
-
     }
 
+    s->seen_sof = 1;
+
     if ((s->rgb && !s->lossless && !s->ls) ||
         (!s->rgb && s->ls && s->nb_components > 1) ||
         (s->avctx->pix_fmt == AV_PIX_FMT_PAL8 && !s->ls)
@@ -764,18 +755,6 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
         memset(s->coefs_finished, 0, sizeof(s->coefs_finished));
     }
 
-    if (s->avctx->hwaccel) {
-        s->hwaccel_picture_private =
-            av_mallocz(s->avctx->hwaccel->frame_priv_data_size);
-        if (!s->hwaccel_picture_private)
-            return AVERROR(ENOMEM);
-
-        ret = s->avctx->hwaccel->start_frame(s->avctx, s->raw_image_buffer,
-                                             s->raw_image_buffer_size);
-        if (ret < 0)
-            return ret;
-    }
-
     return 0;
 }
 
@@ -1630,12 +1609,44 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s, const uint8_t *mb_bitmask,
     const int block_size = s->lossless ? 1 : 8;
     int ilv, prev_shift;
 
-    if (!s->got_picture) {
+    if (!s->seen_sof) {
         av_log(s->avctx, AV_LOG_WARNING,
                 "Can not process SOS before SOF, skipping\n");
         return -1;
     }
 
+    if (!s->got_picture || !s->interlaced || !(s->bottom_field == !s->interlace_polarity)) {
+        av_frame_unref(s->picture_ptr);
+        if (ff_get_buffer(s->avctx, s->picture_ptr, AV_GET_BUFFER_FLAG_REF) < 0)
+            return -1;
+        s->picture_ptr->pict_type = AV_PICTURE_TYPE_I;
+        s->picture_ptr->key_frame = 1;
+
+        for (i = 0; i < 4; i++)
+            s->linesize[i] = s->picture_ptr->linesize[i] << s->interlaced;
+
+        if (s->picture_ptr->format == AV_PIX_FMT_PAL8)
+            memcpy(s->picture_ptr->data[1], s->palette, sizeof(s->palette));
+
+        s->got_picture = 1;
+
+        ff_dlog(s->avctx, "%d %d %d %d %d %d\n",
+                s->width, s->height, s->linesize[0], s->linesize[1],
+                s->interlaced, s->avctx->height);
+    }
+
+    if (s->avctx->hwaccel && !s->hwaccel_picture_private) {
+        s->hwaccel_picture_private =
+            av_mallocz(s->avctx->hwaccel->frame_priv_data_size);
+        if (!s->hwaccel_picture_private)
+            return AVERROR(ENOMEM);
+
+        ret = s->avctx->hwaccel->start_frame(s->avctx, s->raw_image_buffer,
+                                             s->raw_image_buffer_size);
+        if (ret < 0)
+            return ret;
+    }
+
     if (reference) {
         if (reference->width  != s->picture_ptr->width  ||
             reference->height != s->picture_ptr->height ||
@@ -2561,6 +2572,7 @@ eoi_parser:
                     break;
             }
             if (avctx->skip_frame == AVDISCARD_ALL) {
+                s->seen_sof = 0;
                 s->got_picture = 0;
                 ret = AVERROR(EAGAIN);
                 goto the_end_no_picture;
@@ -2574,6 +2586,7 @@ eoi_parser:
             }
             if ((ret = av_frame_ref(frame, s->picture_ptr)) < 0)
                 return ret;
+            s->seen_sof = 0;
             s->got_picture = 0;
 
             frame->pkt_dts = s->pkt->dts;
@@ -2634,6 +2647,7 @@ skip:
     av_log(avctx, AV_LOG_FATAL, "No JPEG data found in image\n");
     return AVERROR_INVALIDDATA;
 fail:
+    s->seen_sof = 0;
     s->got_picture = 0;
     return ret;
 the_end:
@@ -2924,6 +2938,7 @@ av_cold int ff_mjpeg_decode_end(AVCodecContext *avctx)
 static void decode_flush(AVCodecContext *avctx)
 {
     MJpegDecodeContext *s = avctx->priv_data;
+    s->seen_sof = 0;
     s->got_picture = 0;
 
     s->smv_next_frame = 0;
diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
index 2400a179f1..71cacb0b27 100644
--- a/libavcodec/mjpegdec.h
+++ b/libavcodec/mjpegdec.h
@@ -109,6 +109,7 @@ typedef struct MJpegDecodeContext {
     int last_dc[MAX_COMPONENTS]; /* last DEQUANTIZED dc (XXX: am I right to do that ?) */
     AVFrame *picture; /* picture structure */
     AVFrame *picture_ptr; /* pointer to picture structure */
+    int seen_sof;                                   ///< we found a SOF.
     int got_picture;                                ///< we found a SOF and picture is valid, too.
     int linesize[MAX_COMPONENTS];                   ///< linesize << interlaced
     int8_t *qscale_table;
@@ -165,7 +166,9 @@ typedef struct MJpegDecodeContext {
     enum AVPixelFormat hwaccel_sw_pix_fmt;
     enum AVPixelFormat hwaccel_pix_fmt;
     void *hwaccel_picture_private;
+
     struct JLSState *jls_state;
+    uint32_t palette[AVPALETTE_COUNT];
 } MJpegDecodeContext;
 
 int ff_mjpeg_build_vlc(VLC *vlc, const uint8_t *bits_table,
diff --git a/libavcodec/mxpegdec.c b/libavcodec/mxpegdec.c
index 763ce5871d..617da52cf0 100644
--- a/libavcodec/mxpegdec.c
+++ b/libavcodec/mxpegdec.c
@@ -197,7 +197,7 @@ static int mxpeg_decode_frame(AVCodecContext *avctx,
     buf_end = buf + buf_size;
     jpg->got_picture = 0;
     s->got_mxm_bitmask = 0;
-    s->got_sof_data = !!s->got_sof_data;
+    jpg->seen_sof = s->got_sof_data = !!s->got_sof_data;
     while (buf_ptr < buf_end) {
         start_code = ff_mjpeg_find_marker(jpg, &buf_ptr, buf_end,
                                           &unescaped_buf_ptr, &unescaped_buf_size);
-- 
2.31.1



More information about the ffmpeg-devel mailing list