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

James Almer jamrial at gmail.com
Wed Apr 21 21:00:24 EEST 2021


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 at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 



More information about the ffmpeg-devel mailing list