| Message ID | HE1PR0101MB2219AAC54B7B6A6758AD8A898F689@HE1PR0101MB2219.eurprd01.prod.exchangelabs.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr | expand |
| 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 |
On Mon, Aug 15, 2022 at 01:49:24PM +0200, Andreas Rheinhardt wrote: > It is done later in ff_mpv_frame_start() (and nobody uses > current_picture_ptr between setting it in ff_mpv_frame_start()). > > (The reason the vsynth*-h263-obmc code changes is because > the call to ff_find_unused_picture() now happens after the older > pictures have been unreferenced in ff_mpv_frame_start(), > so that their slots in the picture array can be immediately > reused; the obmc code is somehow buggy and changes its output > depending on the earlier contents of the motion_val buffer.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > I'd like to take this opportunity to once again ask anyone familiar > with H.263 to take a look at this OBMC issue. Iam too busy ATM :( i might look at some point but its not very high on my todo as it works :) security & release is more important but i can say this breaks fate as it is: --- ./tests/ref/vsynth/vsynth1-h263-obmc 2022-08-16 00:19:00.345967181 +0200 +++ tests/data/fate/vsynth1-h263-obmc 2022-08-16 00:19:05.262017999 +0200 @@ -1,4 +1,4 @@ 7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi 657320 tests/data/fate/vsynth1-h263-obmc.avi -f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo -stddev: 8.12 PSNR: 29.93 MAXDIFF: 113 bytes: 7603200/ 7603200 +2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo +stddev: 8.38 PSNR: 29.66 MAXDIFF: 116 bytes: 7603200/ 7603200 Test vsynth1-h263-obmc failed. Look at tests/data/fate/vsynth1-h263-obmc.err for details. tests/Makefile:304: recipe for target 'fate-vsynth1-h263-obmc' failed make: *** [fate-vsynth1-h263-obmc] Error 1 [...]
Michael Niedermayer: > On Mon, Aug 15, 2022 at 01:49:24PM +0200, Andreas Rheinhardt wrote: >> It is done later in ff_mpv_frame_start() (and nobody uses >> current_picture_ptr between setting it in ff_mpv_frame_start()). >> >> (The reason the vsynth*-h263-obmc code changes is because >> the call to ff_find_unused_picture() now happens after the older >> pictures have been unreferenced in ff_mpv_frame_start(), >> so that their slots in the picture array can be immediately >> reused; the obmc code is somehow buggy and changes its output >> depending on the earlier contents of the motion_val buffer.) >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- > >> I'd like to take this opportunity to once again ask anyone familiar >> with H.263 to take a look at this OBMC issue. > > Iam too busy ATM :( i might look at some point but its not very high > on my todo as it works :) security & release is more important > Good that you became aware of this issue. (You were the "anyone familiar with H.263" person I thought of.) > but i can say this breaks fate as it is: > I am aware of this; this is due to a slight conflict with b645138a34321fb1d1b7988cd0d78b897e4d65ca. The ref file changes were appropriate for git master at the time I sent this. I already updated mine locally. > --- ./tests/ref/vsynth/vsynth1-h263-obmc 2022-08-16 00:19:00.345967181 +0200 > +++ tests/data/fate/vsynth1-h263-obmc 2022-08-16 00:19:05.262017999 +0200 > @@ -1,4 +1,4 @@ > 7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi > 657320 tests/data/fate/vsynth1-h263-obmc.avi > -f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo > -stddev: 8.12 PSNR: 29.93 MAXDIFF: 113 bytes: 7603200/ 7603200 > +2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo > +stddev: 8.38 PSNR: 29.66 MAXDIFF: 116 bytes: 7603200/ 7603200 > Test vsynth1-h263-obmc failed. Look at tests/data/fate/vsynth1-h263-obmc.err for details. > tests/Makefile:304: recipe for target 'fate-vsynth1-h263-obmc' failed > make: *** [fate-vsynth1-h263-obmc] Error 1 > > [...]
Michael Niedermayer: > On Mon, Aug 15, 2022 at 01:49:24PM +0200, Andreas Rheinhardt wrote: >> It is done later in ff_mpv_frame_start() (and nobody uses >> current_picture_ptr between setting it in ff_mpv_frame_start()). >> >> (The reason the vsynth*-h263-obmc code changes is because >> the call to ff_find_unused_picture() now happens after the older >> pictures have been unreferenced in ff_mpv_frame_start(), >> so that their slots in the picture array can be immediately >> reused; the obmc code is somehow buggy and changes its output >> depending on the earlier contents of the motion_val buffer.) >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- > >> I'd like to take this opportunity to once again ask anyone familiar >> with H.263 to take a look at this OBMC issue. > > Iam too busy ATM :( i might look at some point but its not very high > on my todo as it works :) security & release is more important > If you (and no one else) don't object, I'll apply this tomorrow. > but i can say this breaks fate as it is: > > --- ./tests/ref/vsynth/vsynth1-h263-obmc 2022-08-16 00:19:00.345967181 +0200 > +++ tests/data/fate/vsynth1-h263-obmc 2022-08-16 00:19:05.262017999 +0200 > @@ -1,4 +1,4 @@ > 7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi > 657320 tests/data/fate/vsynth1-h263-obmc.avi > -f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo > -stddev: 8.12 PSNR: 29.93 MAXDIFF: 113 bytes: 7603200/ 7603200 > +2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo > +stddev: 8.38 PSNR: 29.66 MAXDIFF: 116 bytes: 7603200/ 7603200 > Test vsynth1-h263-obmc failed. Look at tests/data/fate/vsynth1-h263-obmc.err for details. > tests/Makefile:304: recipe for target 'fate-vsynth1-h263-obmc' failed > make: *** [fate-vsynth1-h263-obmc] Error 1 > > [...] >
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c index 8db0eccd89..a65f16caea 100644 --- a/libavcodec/h263dec.c +++ b/libavcodec/h263dec.c @@ -543,13 +543,6 @@ retry: return ret; } - if (!s->current_picture_ptr || s->current_picture_ptr->f->data[0]) { - int i = ff_find_unused_picture(s->avctx, s->picture, 0); - if (i < 0) - return i; - s->current_picture_ptr = &s->picture[i]; - } - avctx->has_b_frames = !s->low_delay; if (CONFIG_MPEG4_DECODER && avctx->codec_id == AV_CODEC_ID_MPEG4) { diff --git a/tests/ref/vsynth/vsynth1-h263-obmc b/tests/ref/vsynth/vsynth1-h263-obmc index b7a267a8cb..a9d0b166cf 100644 --- a/tests/ref/vsynth/vsynth1-h263-obmc +++ b/tests/ref/vsynth/vsynth1-h263-obmc @@ -1,4 +1,4 @@ 7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi 657320 tests/data/fate/vsynth1-h263-obmc.avi -844f7ee27fa122e199fe20987b41a15c *tests/data/fate/vsynth1-h263-obmc.out.rawvideo -stddev: 8.16 PSNR: 29.89 MAXDIFF: 113 bytes: 7603200/ 7603200 +f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo +stddev: 8.12 PSNR: 29.93 MAXDIFF: 113 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth2-h263-obmc b/tests/ref/vsynth/vsynth2-h263-obmc index 2cef7f551b..2275b6e446 100644 --- a/tests/ref/vsynth/vsynth2-h263-obmc +++ b/tests/ref/vsynth/vsynth2-h263-obmc @@ -1,4 +1,4 @@ 2d8a58b295e03f94e6a41468b2d3909e *tests/data/fate/vsynth2-h263-obmc.avi 208522 tests/data/fate/vsynth2-h263-obmc.avi -4a939ef99fc759293f2e609bfcacd2a4 *tests/data/fate/vsynth2-h263-obmc.out.rawvideo -stddev: 6.10 PSNR: 32.41 MAXDIFF: 90 bytes: 7603200/ 7603200 +20c4dda7bc5b4da28611a8c731cfa1c5 *tests/data/fate/vsynth2-h263-obmc.out.rawvideo +stddev: 6.08 PSNR: 32.44 MAXDIFF: 81 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth_lena-h263-obmc b/tests/ref/vsynth/vsynth_lena-h263-obmc index 5b963107f6..a18ef8e9e3 100644 --- a/tests/ref/vsynth/vsynth_lena-h263-obmc +++ b/tests/ref/vsynth/vsynth_lena-h263-obmc @@ -1,4 +1,4 @@ 3c6946f808412ac320be9e0c36051ea2 *tests/data/fate/vsynth_lena-h263-obmc.avi 154730 tests/data/fate/vsynth_lena-h263-obmc.avi -588d992d9d8096da8bdc5027268da914 *tests/data/fate/vsynth_lena-h263-obmc.out.rawvideo -stddev: 5.39 PSNR: 33.49 MAXDIFF: 82 bytes: 7603200/ 7603200 +acc9705f4c9a019c2032a875a6a715ae *tests/data/fate/vsynth_lena-h263-obmc.out.rawvideo +stddev: 5.39 PSNR: 33.50 MAXDIFF: 77 bytes: 7603200/ 7603200
It is done later in ff_mpv_frame_start() (and nobody uses current_picture_ptr between setting it in ff_mpv_frame_start()). (The reason the vsynth*-h263-obmc code changes is because the call to ff_find_unused_picture() now happens after the older pictures have been unreferenced in ff_mpv_frame_start(), so that their slots in the picture array can be immediately reused; the obmc code is somehow buggy and changes its output depending on the earlier contents of the motion_val buffer.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- I'd like to take this opportunity to once again ask anyone familiar with H.263 to take a look at this OBMC issue. libavcodec/h263dec.c | 7 ------- tests/ref/vsynth/vsynth1-h263-obmc | 4 ++-- tests/ref/vsynth/vsynth2-h263-obmc | 4 ++-- tests/ref/vsynth/vsynth_lena-h263-obmc | 4 ++-- 4 files changed, 6 insertions(+), 13 deletions(-)