From: mkver via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
To: ffmpeg-devel@ffmpeg.org
Cc: mkver <code@ffmpeg.org>
Subject: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: Avoid mixing return value and error code, avoid branch (PR #20497)
Date: Fri, 12 Sep 2025 02:27:29 -0000
Message-ID: <175764405053.25.4793704935854434545@463a07221176> (raw)
PR #20497 opened by mkver
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20497
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20497.patch
>From 2fdfade62c03c2b2d6eefc4b52bef1d8a4ffcd38 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Thu, 11 Sep 2025 22:04:53 +0200
Subject: [PATCH 1/6] avcodec/mjpegdec: Remove pointless information from
logmessage
There is no reason to add the address of an element of
MJpegDecodeContext (which makes logmessage nonreproducible).
Also avoid always printing a zero.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mjpegdec.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 3dde759fea..bb5540c8bb 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -822,8 +822,7 @@ static inline int mjpeg_decode_dc(MJpegDecodeContext *s, int dc_index)
code = get_vlc2(&s->gb, s->vlcs[0][dc_index].table, 9, 2);
if (code < 0 || code > 16) {
av_log(s->avctx, AV_LOG_WARNING,
- "mjpeg_decode_dc: bad vlc: %d:%d (%p)\n",
- 0, dc_index, &s->vlcs[0][dc_index]);
+ "mjpeg_decode_dc: bad vlc: %d\n", dc_index);
return 0xfffff;
}
--
2.49.1
>From 7d74683d37069770fb619a6d1989b439eff52687 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Thu, 11 Sep 2025 22:33:32 +0200
Subject: [PATCH 2/6] avcodec/mjpegdec: Avoid superfluous secondary error
message
Up until now, a DC error in decode_block() or decode_dc_progressive()
would lead to a warning from mjpeg_decode_dc() and a (less verbose)
error from the caller. Upgrade the former to an error status (all
callers treat is an error) and remove the latter.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mjpegdec.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index bb5540c8bb..711ea1c140 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -821,7 +821,7 @@ static inline int mjpeg_decode_dc(MJpegDecodeContext *s, int dc_index)
int code;
code = get_vlc2(&s->gb, s->vlcs[0][dc_index].table, 9, 2);
if (code < 0 || code > 16) {
- av_log(s->avctx, AV_LOG_WARNING,
+ av_log(s->avctx, AV_LOG_ERROR,
"mjpeg_decode_dc: bad vlc: %d\n", dc_index);
return 0xfffff;
}
@@ -841,7 +841,6 @@ static int decode_block(MJpegDecodeContext *s, int16_t *block, int component,
/* DC coef */
val = mjpeg_decode_dc(s, dc_index);
if (val == 0xfffff) {
- av_log(s->avctx, AV_LOG_ERROR, "error dc\n");
return AVERROR_INVALIDDATA;
}
val = val * (unsigned)quant_matrix[0] + s->last_dc[component];
@@ -889,7 +888,6 @@ static int decode_dc_progressive(MJpegDecodeContext *s, int16_t *block,
s->bdsp.clear_block(block);
val = mjpeg_decode_dc(s, dc_index);
if (val == 0xfffff) {
- av_log(s->avctx, AV_LOG_ERROR, "error dc\n");
return AVERROR_INVALIDDATA;
}
val = (val * (quant_matrix[0] << Al)) + s->last_dc[component];
--
2.49.1
>From d814b5b0e97165c9dfc882bb4d36a7d4431e61ca Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Fri, 12 Sep 2025 00:13:52 +0200
Subject: [PATCH 3/6] avcodec/mjpegdec: Avoid mixing return value and error
code, avoid branch
mjpeg_decode_dc() currently has a special return value (0xfffff)
for the case of invalid data; this value does not overlap with
the ordinary return values, yet the compiler can't prove this
(at least on x86 where an asm version of NEG_USR32 is used
inside get_xbits()), so the non-error return value has to be checked
for being the special value even if mjpeg_decode_dc() is inlined.
This commit avoids this by separating error code and ordinary return
value. It also means that the ljpeg functions return the proper
error code and not -1 in case of invalid data.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mjpegdec.c | 48 ++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 23 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 711ea1c140..82b82ff44d 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -816,20 +816,18 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
return 0;
}
-static inline int mjpeg_decode_dc(MJpegDecodeContext *s, int dc_index)
+static inline int mjpeg_decode_dc(MJpegDecodeContext *s, int dc_index, int *val)
{
int code;
code = get_vlc2(&s->gb, s->vlcs[0][dc_index].table, 9, 2);
if (code < 0 || code > 16) {
av_log(s->avctx, AV_LOG_ERROR,
"mjpeg_decode_dc: bad vlc: %d\n", dc_index);
- return 0xfffff;
+ return AVERROR_INVALIDDATA;
}
- if (code)
- return get_xbits(&s->gb, code);
- else
- return 0;
+ *val = code ? get_xbits(&s->gb, code) : 0;
+ return 0;
}
/* decode block and dequantize */
@@ -839,10 +837,10 @@ static int decode_block(MJpegDecodeContext *s, int16_t *block, int component,
int code, i, j, level, val;
/* DC coef */
- val = mjpeg_decode_dc(s, dc_index);
- if (val == 0xfffff) {
- return AVERROR_INVALIDDATA;
- }
+ int ret = mjpeg_decode_dc(s, dc_index, &val);
+ if (ret < 0)
+ return ret;
+
val = val * (unsigned)quant_matrix[0] + s->last_dc[component];
s->last_dc[component] = val;
block[0] = av_clip_int16(val);
@@ -886,10 +884,10 @@ static int decode_dc_progressive(MJpegDecodeContext *s, int16_t *block,
{
unsigned val;
s->bdsp.clear_block(block);
- val = mjpeg_decode_dc(s, dc_index);
- if (val == 0xfffff) {
- return AVERROR_INVALIDDATA;
- }
+ int ret = mjpeg_decode_dc(s, dc_index, &val);
+ if (ret < 0)
+ return ret;
+
val = (val * (quant_matrix[0] << Al)) + s->last_dc[component];
s->last_dc[component] = val;
block[0] = val;
@@ -1107,6 +1105,7 @@ static int ljpeg_decode_rgb_scan(MJpegDecodeContext *s, int nb_components, int p
int resync_mb_y = 0;
int resync_mb_x = 0;
int vpred[6];
+ int ret;
if (!s->bayer && s->nb_components < 3)
return AVERROR_INVALIDDATA;
@@ -1179,9 +1178,9 @@ static int ljpeg_decode_rgb_scan(MJpegDecodeContext *s, int nb_components, int p
topleft[i] = top[i];
top[i] = buffer[mb_x][i];
- dc = mjpeg_decode_dc(s, s->dc_index[i]);
- if(dc == 0xFFFFF)
- return -1;
+ ret = mjpeg_decode_dc(s, s->dc_index[i], &dc);
+ if (ret < 0)
+ return ret;
if (!s->bayer || mb_x) {
pred = left[i];
@@ -1275,6 +1274,7 @@ static int ljpeg_decode_yuv_scan(MJpegDecodeContext *s, int predictor,
int bits= (s->bits+7)&~7;
int resync_mb_y = 0;
int resync_mb_x = 0;
+ int ret;
point_transform += bits - s->bits;
mask = ((1 << s->bits) - 1) << point_transform;
@@ -1313,9 +1313,10 @@ static int ljpeg_decode_yuv_scan(MJpegDecodeContext *s, int predictor,
for(j=0; j<n; j++) {
int pred, dc;
- dc = mjpeg_decode_dc(s, s->dc_index[i]);
- if(dc == 0xFFFFF)
- return -1;
+ ret = mjpeg_decode_dc(s, s->dc_index[i], &dc);
+ if (ret < 0)
+ return ret;
+
if ( h * mb_x + x >= s->width
|| v * mb_y + y >= s->height) {
// Nothing to do
@@ -1384,9 +1385,10 @@ static int ljpeg_decode_yuv_scan(MJpegDecodeContext *s, int predictor,
for (j = 0; j < n; j++) {
int pred;
- dc = mjpeg_decode_dc(s, s->dc_index[i]);
- if(dc == 0xFFFFF)
- return -1;
+ ret = mjpeg_decode_dc(s, s->dc_index[i], &dc);
+ if (ret < 0)
+ return ret;
+
if ( h * mb_x + x >= s->width
|| v * mb_y + y >= s->height) {
// Nothing to do
--
2.49.1
>From de976e3b7971532081dc1dce63979f22a33efde2 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Fri, 12 Sep 2025 02:38:23 +0200
Subject: [PATCH 4/6] avcodec/mjpegdec: Move initializing HpelDSPContext to
mxpegdec.c
Only the mxpeg decoder uses it (and the reference/bitmask feature
of ff_mjpeg_decode_sos()). So only initialize it for mxpeg which
allows to remove the mjpeg->hpeldsp dependency.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
configure | 4 ++--
libavcodec/mjpegdec.c | 1 -
libavcodec/mxpegdec.c | 3 +++
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index c6afcd7494..4c5bcb5db0 100755
--- a/configure
+++ b/configure
@@ -3047,9 +3047,9 @@ mdec_decoder_select="blockdsp bswapdsp idctdsp"
media100_decoder_select="media100_to_mjpegb_bsf mjpegb_decoder"
metasound_decoder_select="lsp sinewin"
mimic_decoder_select="blockdsp bswapdsp hpeldsp idctdsp"
-mjpeg_decoder_select="blockdsp hpeldsp idctdsp jpegtables"
+mjpeg_decoder_select="blockdsp idctdsp jpegtables"
mjpeg_encoder_select="jpegtables mpegvideoenc"
-mjpegb_decoder_select="mjpeg_decoder"
+mjpegb_decoder_select="hpeldsp mjpeg_decoder"
mlp_decoder_select="mlp_parser"
mlp_encoder_select="lpc audio_frame_queue"
mobiclip_decoder_select="bswapdsp golomb"
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 82b82ff44d..4b3d1e2162 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -134,7 +134,6 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx)
s->avctx = avctx;
ff_blockdsp_init(&s->bdsp);
- ff_hpeldsp_init(&s->hdsp, avctx->flags);
init_idct(avctx);
s->buffer_size = 0;
s->buffer = NULL;
diff --git a/libavcodec/mxpegdec.c b/libavcodec/mxpegdec.c
index 8661bba566..8054d016be 100644
--- a/libavcodec/mxpegdec.c
+++ b/libavcodec/mxpegdec.c
@@ -28,6 +28,7 @@
#include "libavutil/mem.h"
#include "codec_internal.h"
#include "decode.h"
+#include "hpeldsp.h"
#include "mjpeg.h"
#include "mjpegdec.h"
@@ -65,6 +66,8 @@ static av_cold int mxpeg_decode_init(AVCodecContext *avctx)
{
MXpegDecodeContext *s = avctx->priv_data;
+ ff_hpeldsp_init(&s->jpg.hdsp, avctx->flags);
+
s->picture[0] = av_frame_alloc();
s->picture[1] = av_frame_alloc();
if (!s->picture[0] || !s->picture[1])
--
2.49.1
>From 718fd869909126a3e2be890292a36d25662fb638 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Fri, 12 Sep 2025 03:47:13 +0200
Subject: [PATCH 5/6] avcodec/mjpegdec: Avoid using HpelDSPContext
It is quite big and we only need one function from it.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mjpegdec.c | 2 +-
libavcodec/mjpegdec.h | 2 +-
libavcodec/mxpegdec.c | 4 +++-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 4b3d1e2162..5c6ca4bdbf 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1428,7 +1428,7 @@ static av_always_inline void mjpeg_copy_block(MJpegDecodeContext *s,
int linesize, int lowres)
{
switch (lowres) {
- case 0: s->hdsp.put_pixels_tab[1][0](dst, src, linesize, 8);
+ case 0: s->copy_block(dst, src, linesize, 8);
break;
case 1: copy_block4(dst, src, linesize, linesize, 4);
break;
diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
index 8b9ed67856..ce688482cf 100644
--- a/libavcodec/mjpegdec.h
+++ b/libavcodec/mjpegdec.h
@@ -119,8 +119,8 @@ typedef struct MJpegDecodeContext {
int force_pal8;
uint8_t permutated_scantable[64];
BlockDSPContext bdsp;
- HpelDSPContext hdsp;
IDCTDSPContext idsp;
+ op_pixels_func copy_block; ///< only set and used by mxpeg
int restart_interval;
int restart_count;
diff --git a/libavcodec/mxpegdec.c b/libavcodec/mxpegdec.c
index 8054d016be..9a5110a814 100644
--- a/libavcodec/mxpegdec.c
+++ b/libavcodec/mxpegdec.c
@@ -65,8 +65,10 @@ static av_cold int mxpeg_decode_end(AVCodecContext *avctx)
static av_cold int mxpeg_decode_init(AVCodecContext *avctx)
{
MXpegDecodeContext *s = avctx->priv_data;
+ HpelDSPContext hdsp;
- ff_hpeldsp_init(&s->jpg.hdsp, avctx->flags);
+ ff_hpeldsp_init(&hdsp, avctx->flags);
+ s->jpg.copy_block = hdsp.put_pixels_tab[1][0];
s->picture[0] = av_frame_alloc();
s->picture[1] = av_frame_alloc();
--
2.49.1
>From 90cb8e0276ac5666edd2874ba5b55efc74dd0950 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Fri, 12 Sep 2025 04:03:07 +0200
Subject: [PATCH 6/6] avcodec/mjpegdec: Move reference dimension check to
mxpegdec.c
Only the mxpeg sets reference at all.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mjpegdec.c | 9 ---------
libavcodec/mxpegdec.c | 6 ++++++
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 5c6ca4bdbf..6ff0ba5a0b 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1679,15 +1679,6 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s, const uint8_t *mb_bitmask,
return -1;
}
- if (reference) {
- if (reference->width != s->picture_ptr->width ||
- reference->height != s->picture_ptr->height ||
- reference->format != s->picture_ptr->format) {
- av_log(s->avctx, AV_LOG_ERROR, "Reference mismatching\n");
- return AVERROR_INVALIDDATA;
- }
- }
-
/* XXX: verify len field validity */
len = get_bits(&s->gb, 16);
nb_components = get_bits(&s->gb, 8);
diff --git a/libavcodec/mxpegdec.c b/libavcodec/mxpegdec.c
index 9a5110a814..8861a3969d 100644
--- a/libavcodec/mxpegdec.c
+++ b/libavcodec/mxpegdec.c
@@ -179,6 +179,12 @@ static int mxpeg_check_dimensions(MXpegDecodeContext *s, MJpegDecodeContext *jpg
return AVERROR(EINVAL);
}
}
+ if (reference_ptr->width != jpg->picture_ptr->width ||
+ reference_ptr->height != jpg->picture_ptr->height ||
+ reference_ptr->format != jpg->picture_ptr->format) {
+ av_log(jpg->avctx, AV_LOG_ERROR, "Reference mismatching\n");
+ return AVERROR_INVALIDDATA;
+ }
}
return 0;
--
2.49.1
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
reply other threads:[~2025-09-12 2:28 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=175764405053.25.4793704935854434545@463a07221176 \
--to=ffmpeg-devel@ffmpeg.org \
--cc=code@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git