[FFmpeg-devel] [PATCH 2/4] Revert "avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker"
James Almer
jamrial at gmail.com
Sun May 2 17:27:48 EEST 2021
On 5/2/2021 10:59 AM, Michael Niedermayer wrote:
> This reverts commit c8197f73e684b0edc450f3dc2b2b4b3fb9dedd0d.
Applying this alone will break fate-jpegls, which is not good for
bisectabiliy.
Can you merge it with patch 3/4 before pushing? Or temporarily disable
the tests in a separate commit before applying this one if you want 3/4
to be standalone, so the actual fix is readable in git history instead
of lost merged with a revert.
> ---
> libavcodec/jpeglsdec.c | 6 ++-
> libavcodec/mjpegbdec.c | 1 -
> libavcodec/mjpegdec.c | 83 +++++++++++++++++-------------------------
> libavcodec/mjpegdec.h | 3 --
> libavcodec/mxpegdec.c | 2 +-
> 5 files changed, 39 insertions(+), 56 deletions(-)
>
> diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
> index bf51d63c3a..d79bbe1ee3 100644
> --- a/libavcodec/jpeglsdec.c
> +++ b/libavcodec/jpeglsdec.c
> @@ -108,8 +108,9 @@ 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) {
> - uint32_t *pal = s->palette;
> + 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];
> int shift = 0;
>
> if (s->avctx->bits_per_raw_sample > 0 && s->avctx->bits_per_raw_sample < 8) {
> @@ -117,6 +118,7 @@ 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 abc607176a..87eebb8771 100644
> --- a/libavcodec/mjpegbdec.c
> +++ b/libavcodec/mjpegbdec.c
> @@ -55,7 +55,6 @@ 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 16aed078f6..7c66ff8637 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -138,7 +138,6 @@ 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;
> @@ -430,7 +429,6 @@ 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 */
> @@ -683,13 +681,11 @@ 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->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
> + } 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
> s->avctx->pix_fmt = AV_PIX_FMT_GRAY16;
> }
>
> @@ -723,12 +719,25 @@ 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->seen_sof = 1;
> + s->got_picture = 1;
> return 0;
> }
> - }
>
> - s->seen_sof = 1;
> + 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) ||
> (!s->rgb && s->ls && s->nb_components > 1) ||
> @@ -755,6 +764,18 @@ 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;
> }
>
> @@ -1609,44 +1630,12 @@ 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->seen_sof) {
> + if (!s->got_picture) {
> 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 ||
> @@ -2572,7 +2561,6 @@ 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;
> @@ -2586,7 +2574,6 @@ 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;
> @@ -2647,7 +2634,6 @@ 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:
> @@ -2938,7 +2924,6 @@ 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 71cacb0b27..2400a179f1 100644
> --- a/libavcodec/mjpegdec.h
> +++ b/libavcodec/mjpegdec.h
> @@ -109,7 +109,6 @@ 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;
> @@ -166,9 +165,7 @@ 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 8283a3976a..7c97a9340e 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;
> - jpg->seen_sof = s->got_sof_data = !!s->got_sof_data;
> + 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);
>
More information about the ffmpeg-devel
mailing list