diff mbox series

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

Message ID 20210421174055.65029-2-jamrial@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,1/2] fate: add tests for JPEG-LS PAL8 | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate fail Make fate failed
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate warning Make fate failed

Commit Message

James Almer April 21, 2021, 5:40 p.m. UTC 
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@gmail.com>
---
With this, the removal of AV_PIX_FMT_FLAG_PSEUDOPAL will no longer generate
segfauls.
I can't test hwaccels like vaapi to ensure things still work as inteded, seeing
i also had to move the call to start_frame().

Better solutions are very much welcome.

 libavcodec/jpeglsdec.c |  6 ++--
 libavcodec/mjpegdec.c  | 72 +++++++++++++++++++++++-------------------
 libavcodec/mjpegdec.h  |  2 ++
 3 files changed, 43 insertions(+), 37 deletions(-)

Comments

Andreas Rheinhardt April 21, 2021, 5:52 p.m. UTC | #1 
James Almer:
> 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@gmail.com>
> ---
> With this, the removal of AV_PIX_FMT_FLAG_PSEUDOPAL will no longer generate
> segfauls.
> I can't test hwaccels like vaapi to ensure things still work as inteded, seeing
> i also had to move the call to start_frame().
> 
> Better solutions are very much welcome.
> 
>  libavcodec/jpeglsdec.c |  6 ++--
>  libavcodec/mjpegdec.c  | 72 +++++++++++++++++++++++-------------------
>  libavcodec/mjpegdec.h  |  2 ++
>  3 files changed, 43 insertions(+), 37 deletions(-)
> 
> diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
> index d79bbe1ee3..e3ffec59bf 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 = (uint32_t *)s->palette;

There is nothing guaranteeing proper alignment for s->palette. Better
use an uint32_t[AVPALETTE_COUNT] for it.

>              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/mjpegdec.c b/libavcodec/mjpegdec.c
> index 7c66ff8637..cf65955891 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -681,11 +681,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((uint32_t *)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;
>          }
>  
> @@ -716,27 +718,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>              s->avctx->pix_fmt     = s->hwaccel_pix_fmt;
>          }
>  
> +        s->got_picture            = 1;
> +
>          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;
>              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);
> -
>      }
>  
>      if ((s->rgb && !s->lossless && !s->ls) ||
> @@ -764,18 +752,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;
>  }
>  
> @@ -1636,6 +1612,36 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s, const uint8_t *mb_bitmask,
>          return -1;
>      }
>  
> +    if (!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));
> +
> +        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 ||
> diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
> index 2400a179f1..f66bdf0cd9 100644
> --- a/libavcodec/mjpegdec.h
> +++ b/libavcodec/mjpegdec.h
> @@ -165,7 +165,9 @@ typedef struct MJpegDecodeContext {
>      enum AVPixelFormat hwaccel_sw_pix_fmt;
>      enum AVPixelFormat hwaccel_pix_fmt;
>      void *hwaccel_picture_private;
> +
>      struct JLSState *jls_state;
> +    uint8_t palette[AVPALETTE_SIZE];
>  } MJpegDecodeContext;
>  
>  int ff_mjpeg_build_vlc(VLC *vlc, const uint8_t *bits_table,
>
James Almer April 21, 2021, 6 p.m. UTC | #2 
On 4/21/2021 2:52 PM, Andreas Rheinhardt wrote:
> James Almer:
>> 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@gmail.com>
>> ---
>> With this, the removal of AV_PIX_FMT_FLAG_PSEUDOPAL will no longer generate
>> segfauls.
>> I can't test hwaccels like vaapi to ensure things still work as inteded, seeing
>> i also had to move the call to start_frame().
>>
>> Better solutions are very much welcome.
>>
>>   libavcodec/jpeglsdec.c |  6 ++--
>>   libavcodec/mjpegdec.c  | 72 +++++++++++++++++++++++-------------------
>>   libavcodec/mjpegdec.h  |  2 ++
>>   3 files changed, 43 insertions(+), 37 deletions(-)
>>
>> diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
>> index d79bbe1ee3..e3ffec59bf 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 = (uint32_t *)s->palette;
> 
> There is nothing guaranteeing proper alignment for s->palette. Better
> use an uint32_t[AVPALETTE_COUNT] for it.

Changed locally. Thanks.

> 
>>               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/mjpegdec.c b/libavcodec/mjpegdec.c
>> index 7c66ff8637..cf65955891 100644
>> --- a/libavcodec/mjpegdec.c
>> +++ b/libavcodec/mjpegdec.c
>> @@ -681,11 +681,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((uint32_t *)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;
>>           }
>>   
>> @@ -716,27 +718,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>>               s->avctx->pix_fmt     = s->hwaccel_pix_fmt;
>>           }
>>   
>> +        s->got_picture            = 1;
>> +
>>           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;
>>               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);
>> -
>>       }
>>   
>>       if ((s->rgb && !s->lossless && !s->ls) ||
>> @@ -764,18 +752,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;
>>   }
>>   
>> @@ -1636,6 +1612,36 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s, const uint8_t *mb_bitmask,
>>           return -1;
>>       }
>>   
>> +    if (!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));
>> +
>> +        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 ||
>> diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
>> index 2400a179f1..f66bdf0cd9 100644
>> --- a/libavcodec/mjpegdec.h
>> +++ b/libavcodec/mjpegdec.h
>> @@ -165,7 +165,9 @@ typedef struct MJpegDecodeContext {
>>       enum AVPixelFormat hwaccel_sw_pix_fmt;
>>       enum AVPixelFormat hwaccel_pix_fmt;
>>       void *hwaccel_picture_private;
>> +
>>       struct JLSState *jls_state;
>> +    uint8_t palette[AVPALETTE_SIZE];
>>   } MJpegDecodeContext;
>>   
>>   int ff_mjpeg_build_vlc(VLC *vlc, const uint8_t *bits_table,
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Michael Niedermayer April 22, 2021, 1:01 p.m. UTC | #3 
On Wed, Apr 21, 2021 at 02:40:55PM -0300, James Almer wrote:
> 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@gmail.com>
> ---
> With this, the removal of AV_PIX_FMT_FLAG_PSEUDOPAL will no longer generate
> segfauls.
> I can't test hwaccels like vaapi to ensure things still work as inteded, seeing
> i also had to move the call to start_frame().
> 
> Better solutions are very much welcome.
> 
>  libavcodec/jpeglsdec.c |  6 ++--
>  libavcodec/mjpegdec.c  | 72 +++++++++++++++++++++++-------------------
>  libavcodec/mjpegdec.h  |  2 ++
>  3 files changed, 43 insertions(+), 37 deletions(-)

this causes a change in one timestamp on decode of:

./ffmpeg -ss 1.2 -i ~/tickets/3245/bad-example.mov -vframes 10 outnew.avi
./ffmpeg -i out.avi -f framecrc /tmp/crc

--- /tmp/crc	2021-04-22 14:56:47.981354127 +0200
+++ /tmp/crcnew	2021-04-22 14:56:32.373271448 +0200
@@ -23,8 +23,8 @@
 0,          4,          4,        1,   622080, 0xf24584d3
 1,       8064,       8064,     1152,     4608, 0x70bf8dc7
 1,       9216,       9216,     1152,     4608, 0xd00f873a
+0,          5,          5,        1,   622080, 0x5f8afe7d
 1,      10368,      10368,     1152,     4608, 0xd45d84b8
-0,          6,          6,        1,   622080, 0x5f8afe7d
 1,      11520,      11520,     1152,     4608, 0x64b5866e
 1,      12672,      12672,     1152,     4608, 0x36a18daf
 0,          7,          7,        1,   622080, 0x09918d93

 
I did not investigate why or if that is even a issue, just wanted to
report it

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
index d79bbe1ee3..e3ffec59bf 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 = (uint32_t *)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/mjpegdec.c b/libavcodec/mjpegdec.c
index 7c66ff8637..cf65955891 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -681,11 +681,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((uint32_t *)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;
         }
 
@@ -716,27 +718,13 @@  int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
             s->avctx->pix_fmt     = s->hwaccel_pix_fmt;
         }
 
+        s->got_picture            = 1;
+
         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;
             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);
-
     }
 
     if ((s->rgb && !s->lossless && !s->ls) ||
@@ -764,18 +752,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;
 }
 
@@ -1636,6 +1612,36 @@  int ff_mjpeg_decode_sos(MJpegDecodeContext *s, const uint8_t *mb_bitmask,
         return -1;
     }
 
+    if (!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));
+
+        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 ||
diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
index 2400a179f1..f66bdf0cd9 100644
--- a/libavcodec/mjpegdec.h
+++ b/libavcodec/mjpegdec.h
@@ -165,7 +165,9 @@  typedef struct MJpegDecodeContext {
     enum AVPixelFormat hwaccel_sw_pix_fmt;
     enum AVPixelFormat hwaccel_pix_fmt;
     void *hwaccel_picture_private;
+
     struct JLSState *jls_state;
+    uint8_t palette[AVPALETTE_SIZE];
 } MJpegDecodeContext;
 
 int ff_mjpeg_build_vlc(VLC *vlc, const uint8_t *bits_table,