diff mbox series

[FFmpeg-devel,1/3] avcodec/h264_slice: Fix decoding undamaged input with slices

Message ID AS1PR01MB95645B7A1124DEDE589F0EC48F199@AS1PR01MB9564.eurprd01.prod.exchangelabs.com
State Accepted
Commit c4fcfa47df24bbef354ae3ae3e1fe839c95519fb
Headers show
Series [FFmpeg-devel,1/3] avcodec/h264_slice: Fix decoding undamaged input with slices | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt March 24, 2022, 8:20 p.m. UTC
ff_er_frame_start() initializes ERContext.error_count
to three times the number of macroblocks to decode.
Later ff_er_add_slice() reduces this number by the amount
of macroblocks whose AC resp. DC resp. MV have been finished
(so every correctly decoded MB counts three times).
So the frame has been decoded correctly if error_count is zero
at the end.

The H.264 decoder uses multiple ERContexts when using
slice threading and therefore combines these error counts:
The first slice's ERContext is intended to be initialized
by ff_er_frame_start(), error_count of all the other
slice contexts is intended to be zeroed initially and
all afterwards all the error_counts are summed.

Yet commit 43b434210e597d484aef57c4139c3126d22b7e2b
(probably unintentionally) changed the code to set
the first slice's error_count to zero as well.
This leads to bogus error messages in case one decodes
an input video using multiple slices with slice threading
with error concealment enabled (which is not the default)
("concealing 0 DC, 0 AC, 0 MV errors in [IPB] frame");
furthermore the returned frame is marked as corrupt as well
(ffmpeg reports "corrupt decoded frame in stream %d" for this).

This can be fixed easily given that only the first ERContext
is really used since 7be2d2a70cd20d88fd826a83f87037d14681a579:
Don't reset the error_count; and don't sum the error counts as well.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/h264_slice.c | 7 -------
 1 file changed, 7 deletions(-)

Comments

Michael Niedermayer March 25, 2022, 5:50 p.m. UTC | #1
On Thu, Mar 24, 2022 at 09:20:06PM +0100, Andreas Rheinhardt wrote:
> ff_er_frame_start() initializes ERContext.error_count
> to three times the number of macroblocks to decode.
> Later ff_er_add_slice() reduces this number by the amount
> of macroblocks whose AC resp. DC resp. MV have been finished
> (so every correctly decoded MB counts three times).
> So the frame has been decoded correctly if error_count is zero
> at the end.
> 
> The H.264 decoder uses multiple ERContexts when using
> slice threading and therefore combines these error counts:
> The first slice's ERContext is intended to be initialized
> by ff_er_frame_start(), error_count of all the other
> slice contexts is intended to be zeroed initially and
> all afterwards all the error_counts are summed.
> 
> Yet commit 43b434210e597d484aef57c4139c3126d22b7e2b
> (probably unintentionally) changed the code to set
> the first slice's error_count to zero as well.
> This leads to bogus error messages in case one decodes
> an input video using multiple slices with slice threading
> with error concealment enabled (which is not the default)
> ("concealing 0 DC, 0 AC, 0 MV errors in [IPB] frame");
> furthermore the returned frame is marked as corrupt as well
> (ffmpeg reports "corrupt decoded frame in stream %d" for this).
> 
> This can be fixed easily given that only the first ERContext
> is really used since 7be2d2a70cd20d88fd826a83f87037d14681a579:
> Don't reset the error_count; and don't sum the error counts as well.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/h264_slice.c | 7 -------
>  1 file changed, 7 deletions(-)

this patchset is a nice cleanup/fix to the error concealment code
in h264

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index ee84b3764d..f6104dde89 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -2931,9 +2931,6 @@  int ff_h264_execute_decode_slices(H264Context *h)
             int slice_idx;
 
             sl                 = &h->slice_ctx[i];
-            if (CONFIG_ERROR_RESILIENCE) {
-                sl->er.error_count = 0;
-            }
 
             /* make sure none of those slices overlap */
             slice_idx = sl->mb_y * h->mb_width + sl->mb_x;
@@ -2954,10 +2951,6 @@  int ff_h264_execute_decode_slices(H264Context *h)
         /* pull back stuff from slices to master context */
         sl                   = &h->slice_ctx[context_count - 1];
         h->mb_y              = sl->mb_y;
-        if (CONFIG_ERROR_RESILIENCE) {
-            for (i = 1; i < context_count; i++)
-                h->slice_ctx[0].er.error_count += h->slice_ctx[i].er.error_count;
-        }
 
         if (h->postpone_filter) {
             h->postpone_filter = 0;