[FFmpeg-devel] [PATCH 2/2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Wed Apr 21 20:52:59 EEST 2021
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.
> 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,
>
More information about the ffmpeg-devel
mailing list