Message ID | 20210502135906.12288-3-michael@niedermayer.cc |
---|---|
State | Accepted |
Headers | show |
Series | [FFmpeg-devel,1/4] Revert "avcodec/mjpegdec: fix SOF check in EOI" | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
On 5/2/2021 10:59 AM, Michael Niedermayer wrote: > This simply performs a 2nd pass if a LSE is encountered with GRAY8 > > Fixes: tickets/3933/128.jls > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/jpeglsdec.c | 6 ++++-- > libavcodec/mjpegdec.c | 10 +++++++--- > libavcodec/mjpegdec.h | 1 + > 3 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c > index d79bbe1ee3..bd9224d97d 100644 > --- a/libavcodec/jpeglsdec.c > +++ b/libavcodec/jpeglsdec.c > @@ -118,8 +118,10 @@ 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; > + s->force_pal8 = 1; > + if (!pal) > + return 1; > + > for (i=s->palette_index; i<=maxtab; i++) { Can maxtab be < 255? The palette may need to be zeroed in that case, because get_buffer2 does not require this from implementations. avcodec_default_get_buffer2() does it when allocating buffers in the pool, though, but not after fetching them, so reused buffers may be dirty. They will also be dirty if memory_poisoning is enabled. Alternatively, avpriv_set_systematic_pal2() could be called with GRAY8 as pix_fmt argument, to maintain the pre-bump behavior of setting such initial values. > uint8_t k = i << shift; > pal[k] = 0; > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c > index 7c66ff8637..0691148027 100644 > --- a/libavcodec/mjpegdec.c > +++ b/libavcodec/mjpegdec.c > @@ -582,7 +582,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) > case 0x43000000: > case 0x44000000: > if(s->bits <= 8) > - s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; > + s->avctx->pix_fmt = s->force_pal8 ? AV_PIX_FMT_PAL8 : AV_PIX_FMT_GRAY8; > else > s->avctx->pix_fmt = AV_PIX_FMT_GRAY16; > break; > @@ -681,7 +681,7 @@ 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) > + } else if (s->palette_index && s->bits <= 8 || s->force_pal8) > s->avctx->pix_fmt = AV_PIX_FMT_PAL8; > else if (s->bits <= 8) > s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; > @@ -2398,6 +2398,8 @@ int ff_mjpeg_receive_frame(AVCodecContext *avctx, AVFrame *frame) > int ret = 0; > int is16bit; > > + s->force_pal8 = 0; > + > if (avctx->codec_id == AV_CODEC_ID_SMVJPEG && s->smv_next_frame > 0) > return smv_process_frame(avctx, frame); > > @@ -2411,7 +2413,7 @@ int ff_mjpeg_receive_frame(AVCodecContext *avctx, AVFrame *frame) > ret = mjpeg_get_packet(avctx); > if (ret < 0) > return ret; > - > +redo_for_pal8: > buf_ptr = s->pkt->data; > buf_end = s->pkt->data + s->pkt->size; > while (buf_ptr < buf_end) { > @@ -2542,6 +2544,8 @@ int ff_mjpeg_receive_frame(AVCodecContext *avctx, AVFrame *frame) > if (!CONFIG_JPEGLS_DECODER || > (ret = ff_jpegls_decode_lse(s)) < 0) > goto fail; > + if (ret == 1) > + goto redo_for_pal8; > break; > case EOI: > eoi_parser: > diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h > index 2400a179f1..648dd714e1 100644 > --- a/libavcodec/mjpegdec.h > +++ b/libavcodec/mjpegdec.h > @@ -117,6 +117,7 @@ typedef struct MJpegDecodeContext { > uint8_t *last_nnz[MAX_COMPONENTS]; > uint64_t coefs_finished[MAX_COMPONENTS]; ///< bitmask of which coefs have been completely decoded (progressive mode) > int palette_index; > + int force_pal8; > ScanTable scantable; > BlockDSPContext bdsp; > HpelDSPContext hdsp; >
On Sun, May 02, 2021 at 02:51:20PM -0300, James Almer wrote: > On 5/2/2021 10:59 AM, Michael Niedermayer wrote: > > This simply performs a 2nd pass if a LSE is encountered with GRAY8 > > > > Fixes: tickets/3933/128.jls > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/jpeglsdec.c | 6 ++++-- > > libavcodec/mjpegdec.c | 10 +++++++--- > > libavcodec/mjpegdec.h | 1 + > > 3 files changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c > > index d79bbe1ee3..bd9224d97d 100644 > > --- a/libavcodec/jpeglsdec.c > > +++ b/libavcodec/jpeglsdec.c > > @@ -118,8 +118,10 @@ 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; > > + s->force_pal8 = 1; > > + if (!pal) > > + return 1; > > + > > for (i=s->palette_index; i<=maxtab; i++) { > > Can maxtab be < 255? The palette may need to be zeroed in that case, because > get_buffer2 does not require this from implementations. > avcodec_default_get_buffer2() does it when allocating buffers in the pool, > though, but not after fetching them, so reused buffers may be dirty. They > will also be dirty if memory_poisoning is enabled. will clear the palette after obtaining the picture and will apply the patchset thx [...]
diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c index d79bbe1ee3..bd9224d97d 100644 --- a/libavcodec/jpeglsdec.c +++ b/libavcodec/jpeglsdec.c @@ -118,8 +118,10 @@ 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; + s->force_pal8 = 1; + if (!pal) + return 1; + for (i=s->palette_index; i<=maxtab; i++) { uint8_t k = i << shift; pal[k] = 0; diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index 7c66ff8637..0691148027 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -582,7 +582,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) case 0x43000000: case 0x44000000: if(s->bits <= 8) - s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; + s->avctx->pix_fmt = s->force_pal8 ? AV_PIX_FMT_PAL8 : AV_PIX_FMT_GRAY8; else s->avctx->pix_fmt = AV_PIX_FMT_GRAY16; break; @@ -681,7 +681,7 @@ 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) + } else if (s->palette_index && s->bits <= 8 || s->force_pal8) s->avctx->pix_fmt = AV_PIX_FMT_PAL8; else if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_GRAY8; @@ -2398,6 +2398,8 @@ int ff_mjpeg_receive_frame(AVCodecContext *avctx, AVFrame *frame) int ret = 0; int is16bit; + s->force_pal8 = 0; + if (avctx->codec_id == AV_CODEC_ID_SMVJPEG && s->smv_next_frame > 0) return smv_process_frame(avctx, frame); @@ -2411,7 +2413,7 @@ int ff_mjpeg_receive_frame(AVCodecContext *avctx, AVFrame *frame) ret = mjpeg_get_packet(avctx); if (ret < 0) return ret; - +redo_for_pal8: buf_ptr = s->pkt->data; buf_end = s->pkt->data + s->pkt->size; while (buf_ptr < buf_end) { @@ -2542,6 +2544,8 @@ int ff_mjpeg_receive_frame(AVCodecContext *avctx, AVFrame *frame) if (!CONFIG_JPEGLS_DECODER || (ret = ff_jpegls_decode_lse(s)) < 0) goto fail; + if (ret == 1) + goto redo_for_pal8; break; case EOI: eoi_parser: diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h index 2400a179f1..648dd714e1 100644 --- a/libavcodec/mjpegdec.h +++ b/libavcodec/mjpegdec.h @@ -117,6 +117,7 @@ typedef struct MJpegDecodeContext { uint8_t *last_nnz[MAX_COMPONENTS]; uint64_t coefs_finished[MAX_COMPONENTS]; ///< bitmask of which coefs have been completely decoded (progressive mode) int palette_index; + int force_pal8; ScanTable scantable; BlockDSPContext bdsp; HpelDSPContext hdsp;
This simply performs a 2nd pass if a LSE is encountered with GRAY8 Fixes: tickets/3933/128.jls Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/jpeglsdec.c | 6 ++++-- libavcodec/mjpegdec.c | 10 +++++++--- libavcodec/mjpegdec.h | 1 + 3 files changed, 12 insertions(+), 5 deletions(-)