* [FFmpeg-devel] [PATCH 1/2] avcodec/mpeg12dec: Use saturated addition when combining, error_count
@ 2025-03-14 19:32 Andreas Rheinhardt
2025-03-16 3:51 ` Andreas Rheinhardt
0 siblings, 1 reply; 2+ messages in thread
From: Andreas Rheinhardt @ 2025-03-14 19:32 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1: Type: text/plain, Size: 29 bytes --]
Patches attached.
- Andreas
[-- Attachment #2: 0001-avcodec-mpeg12dec-Use-saturated-addition-when-combin.patch --]
[-- Type: text/x-patch, Size: 4795 bytes --]
From e79b422c0ed3732ec15bd25bdf2ad87f45dbddfd Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Fri, 14 Mar 2025 19:10:57 +0100
Subject: [PATCH 1/2] avcodec/mpeg12dec: Use saturated addition when combining
error_count
Fixes undefined integer overflows. The overflows could always
happen, yet before 4d8b706b1d33e75eb30b289c152280d4535c40e6
it was not undefined because the code implicitly used atomic
types, for which signed integer overflow is defined.
Reported-by: Kacper Michajlow <kasper93@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mpeg12dec.c | 62 ++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index ffe0710470..efb66ef956 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2233,6 +2233,36 @@ static int mpeg_decode_gop(AVCodecContext *avctx,
return 0;
}
+static void mpeg12_execute_slice_threads(AVCodecContext *avctx,
+ Mpeg1Context *const s)
+{
+ if (HAVE_THREADS && (avctx->active_thread_type & FF_THREAD_SLICE) &&
+ !avctx->hwaccel) {
+ MpegEncContext *const s2 = &s->mpeg_enc_ctx;
+ int error_count = 0;
+ av_assert0(avctx->thread_count > 1);
+
+ avctx->execute(avctx, slice_decode_thread,
+ s2->thread_context, NULL,
+ s->slice_count, sizeof(void *));
+
+ for (int i = 0; i < s->slice_count; i++) {
+ MpegEncContext *const slice = s2->thread_context[i];
+ int slice_err = atomic_load_explicit(&slice->er.error_count,
+ memory_order_relaxed);
+ // error_count can get set to INT_MAX on serious errors.
+ // So use saturated addition.
+ if ((unsigned)slice_err > INT_MAX - error_count) {
+ error_count = INT_MAX;
+ break;
+ }
+ error_count += slice_err;
+ }
+ atomic_store_explicit(&s2->er.error_count, error_count,
+ memory_order_relaxed);
+ }
+}
+
static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
int *got_output, const uint8_t *buf, int buf_size)
{
@@ -2250,22 +2280,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
if (start_code > 0x1ff) {
if (!skip_frame) {
- if (HAVE_THREADS &&
- (avctx->active_thread_type & FF_THREAD_SLICE) &&
- !avctx->hwaccel) {
- int error_count = 0;
- int i;
- av_assert0(avctx->thread_count > 1);
-
- avctx->execute(avctx, slice_decode_thread,
- &s2->thread_context[0], NULL,
- s->slice_count, sizeof(void *));
- for (i = 0; i < s->slice_count; i++)
- error_count += atomic_load_explicit(&s2->thread_context[i]->er.error_count,
- memory_order_relaxed);
- atomic_store_explicit(&s2->er.error_count, error_count,
- memory_order_relaxed);
- }
+ mpeg12_execute_slice_threads(avctx, s);
ret = slice_end(avctx, picture, got_output);
if (ret < 0)
@@ -2324,19 +2339,8 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
s2->intra_dc_precision= 3;
s2->intra_matrix[0]= 1;
}
- if (HAVE_THREADS && (avctx->active_thread_type & FF_THREAD_SLICE) &&
- !avctx->hwaccel && s->slice_count) {
- int error_count = 0;
- int i;
-
- avctx->execute(avctx, slice_decode_thread,
- s2->thread_context, NULL,
- s->slice_count, sizeof(void *));
- for (i = 0; i < s->slice_count; i++)
- error_count += atomic_load_explicit(&s2->thread_context[i]->er.error_count,
- memory_order_relaxed);
- atomic_store_explicit(&s2->er.error_count, error_count,
- memory_order_relaxed);
+ if (s->slice_count) {
+ mpeg12_execute_slice_threads(avctx, s);
s->slice_count = 0;
}
if (last_code == 0 || last_code == SLICE_MIN_START_CODE) {
--
2.45.2
[-- Attachment #3: 0002-avcodec-mpeg12dec-Don-t-assert-on-thread_count.patch --]
[-- Type: text/x-patch, Size: 1502 bytes --]
From 12d3af209ed16a136d917f8b12e8547213fef52f Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Date: Fri, 14 Mar 2025 20:18:37 +0100
Subject: [PATCH 2/2] avcodec/mpeg12dec: Don't assert on thread_count
Nothing in this decoder would break if the generic code were to be
changed to allow slice "threading" with only one thread.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mpeg12dec.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index efb66ef956..c36471a8b3 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2240,7 +2240,6 @@ static void mpeg12_execute_slice_threads(AVCodecContext *avctx,
!avctx->hwaccel) {
MpegEncContext *const s2 = &s->mpeg_enc_ctx;
int error_count = 0;
- av_assert0(avctx->thread_count > 1);
avctx->execute(avctx, slice_decode_thread,
s2->thread_context, NULL,
@@ -2538,7 +2537,6 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
int threshold = (s2->mb_height * s->slice_count +
s2->slice_context_count / 2) /
s2->slice_context_count;
- av_assert0(avctx->thread_count > 1);
if (threshold <= mb_y) {
MpegEncContext *thread_context = s2->thread_context[s->slice_count];
--
2.45.2
[-- Attachment #4: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-03-16 3:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-14 19:32 [FFmpeg-devel] [PATCH 1/2] avcodec/mpeg12dec: Use saturated addition when combining, error_count Andreas Rheinhardt
2025-03-16 3:51 ` Andreas Rheinhardt
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