diff mbox series

[FFmpeg-devel,3/4] avcodec/mjpegdec: Decode to PAL8 independant of the location of LSE

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"
Related show

Checks

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

Commit Message

Michael Niedermayer May 2, 2021, 1:59 p.m. UTC
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(-)

Comments

James Almer May 2, 2021, 5:51 p.m. UTC | #1
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;
>
Michael Niedermayer May 3, 2021, 6:59 p.m. UTC | #2
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 mbox series

Patch

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;