On Fri, Jun 7, 2024 at 10:10 PM Ramiro Polla <ramiro.po...@gmail.com> wrote:
> On Fri, Jun 7, 2024 at 9:35 PM Andreas Rheinhardt
> <andreas.rheinha...@outlook.com> wrote:
> > Ramiro Polla:
> > > Do av_clip_int16(val) _after_ copying the value to last_dc.
> > >
> > > Related commits: c28f648b19d and dffae122d0f
> > > Related ticket: 4683
> > > ---
> > > libavcodec/mjpegdec.c | 3 +--
> > > tests/ref/fate/jpg-12bpp | 2 +-
> > > 2 files changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > > index 1481a7f285..7daec649bc 100644
> > > --- a/libavcodec/mjpegdec.c
> > > +++ b/libavcodec/mjpegdec.c
> > > @@ -843,9 +843,8 @@ static int decode_block(MJpegDecodeContext *s,
> > > int16_t *block, int component,
> > > return AVERROR_INVALIDDATA;
> > > }
> > > val = val * (unsigned)quant_matrix[0] + s->last_dc[component];
> > > - val = av_clip_int16(val);
> > > s->last_dc[component] = val;
> > > - block[0] = val;
> > > + block[0] = av_clip_int16(val);
> > > /* AC coefs */
> > > i = 0;
> > > {OPEN_READER(re, &s->gb);
> > > diff --git a/tests/ref/fate/jpg-12bpp b/tests/ref/fate/jpg-12bpp
> > > index b3c662d587..9b039a92c6 100644
> > > --- a/tests/ref/fate/jpg-12bpp
> > > +++ b/tests/ref/fate/jpg-12bpp
> > > @@ -3,4 +3,4 @@
> > > #codec_id 0: rawvideo
> > > #dimensions 0: 999x749
> > > #sar 0: 1/1
> > > -0, 0, 0, 1, 1496502, 0xd91deb4b
> > > +0, 0, 0, 1, 1496502, 0x44efc0af
> >
> > Is the change for the fate-sample supposed to be an improvement or what
> > is the rationale for this? (Is this change mandated by the spec?)
>
> As far as I can tell the only sample we have that triggers this is
> buggy anyways, so it's not something spec-related. It seems more
> correct to me to clip the values that overflow only for the block, and
> not propagate the differences from the clipping to the next dc values.
>
> This change comes from another project where I decouple the bitstream
> reading from the processing. Currently we have this comment in
> MJpegDecodeContext:
> int last_dc[MAX_COMPONENTS]; /* last DEQUANTIZED dc (XXX: am I right
> to do that ?) */
>
> What I do is keep the last quantized dc values as they were read from
> the bitstream, but then I have to add the dc shift for every block.
> Since it incurs one extra add per block, I'm not submitting the entire
> patch, but only this chunk.
Updated patch attached with (hopefully) more descriptive commit message.
If there are no objections, I'll apply this tomorrow.
From 30bbcaf859bb90f0213d0b14284b08afe6933a75 Mon Sep 17 00:00:00 2001
From: Ramiro Polla <ramiro.po...@gmail.com>
Date: Fri, 7 Jun 2024 21:26:54 +0200
Subject: [PATCH] libavcodec/mjpeg: preserve unclipped last_dc value
Perform av_clip_int16(val) _after_ copying the value to last_dc.
This change ensures that clipping is applied only within the context of
the current block, preventing the propagation of clipped values to
subsequent DC components.
Related commits: c28f648b19d and dffae122d0f
Related ticket: 4683
---
libavcodec/mjpegdec.c | 3 +--
tests/ref/fate/jpg-12bpp | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 1481a7f285..7daec649bc 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -843,9 +843,8 @@ static int decode_block(MJpegDecodeContext *s, int16_t *block, int component,
return AVERROR_INVALIDDATA;
}
val = val * (unsigned)quant_matrix[0] + s->last_dc[component];
- val = av_clip_int16(val);
s->last_dc[component] = val;
- block[0] = val;
+ block[0] = av_clip_int16(val);
/* AC coefs */
i = 0;
{OPEN_READER(re, &s->gb);
diff --git a/tests/ref/fate/jpg-12bpp b/tests/ref/fate/jpg-12bpp
index b3c662d587..9b039a92c6 100644
--- a/tests/ref/fate/jpg-12bpp
+++ b/tests/ref/fate/jpg-12bpp
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 999x749
#sar 0: 1/1
-0, 0, 0, 1, 1496502, 0xd91deb4b
+0, 0, 0, 1, 1496502, 0x44efc0af
--
2.30.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
