| Message ID | DB6PR0101MB221462DAF83CE24E63C8027B8F629@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/9] avcodec/wmv2dec: Zero mb_type array for I pictures | 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 |
Andreas Rheinhardt: > Codecs call ff_find_unused_picture() to get the index of > an unused picture; said picture may have buffers left > from using it previously (these buffers are intentionally > not unreferenced so that it might be possible to reuse them; > this is mpegvideo's version of a bufferpool). They should > not make any assumptions about which picture they get. > Yet somehow this is not true when decoding OBMC: Returning > random empty pictures (instead of the first one) leads > to nondeterministic results; similarly, explicitly > rezeroing the buffer before handing it over to the codec > changes the outcome of the h263-obmc tests, but it makes it > independent of the returned pictures. Therefore this commit > does so. > > (No, this commit is not intended to be applied. I just hope > to arouse the interest of people familiar with H.263 > to look at this issue.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- Will apply this patchset minus this patch tomorrow unless there are objections. And I would really like anybody with knowledge about H.263 to take a look at the obmc code to see what is wrong with it. > libavcodec/mpegpicture.c | 4 ++++ > 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, 10 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c > index c57f149752..2192f74cea 100644 > --- a/libavcodec/mpegpicture.c > +++ b/libavcodec/mpegpicture.c > @@ -280,6 +280,10 @@ int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me, > for (i = 0; i < 2; i++) { > pic->motion_val[i] = (int16_t (*)[2])pic->motion_val_buf[i]->data + 4; > pic->ref_index[i] = pic->ref_index_buf[i]->data; > + /* FIXME: The output of H.263 with OBMC depends upon > + * the earlier content of the buffer; therefore we > + * reset it here. */ > + memset(pic->motion_val_buf[i]->data, 0, pic->motion_val_buf[i]->size); > } > } > > diff --git a/tests/ref/vsynth/vsynth1-h263-obmc b/tests/ref/vsynth/vsynth1-h263-obmc > index b7a267a8cb..aed283ed53 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 > +2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo > +stddev: 8.38 PSNR: 29.66 MAXDIFF: 116 bytes: 7603200/ 7603200 > diff --git a/tests/ref/vsynth/vsynth2-h263-obmc b/tests/ref/vsynth/vsynth2-h263-obmc > index 2cef7f551b..c0dcc3239e 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 > +3500b4227c1e6309ca5213414599266f *tests/data/fate/vsynth2-h263-obmc.out.rawvideo > +stddev: 6.19 PSNR: 32.29 MAXDIFF: 111 bytes: 7603200/ 7603200 > diff --git a/tests/ref/vsynth/vsynth_lena-h263-obmc b/tests/ref/vsynth/vsynth_lena-h263-obmc > index 5b963107f6..78d7cc7277 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 > +737af7fb166e2260ba049ae6bc30673d *tests/data/fate/vsynth_lena-h263-obmc.out.rawvideo > +stddev: 5.42 PSNR: 33.44 MAXDIFF: 77 bytes: 7603200/ 7603200
diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c index c57f149752..2192f74cea 100644 --- a/libavcodec/mpegpicture.c +++ b/libavcodec/mpegpicture.c @@ -280,6 +280,10 @@ int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me, for (i = 0; i < 2; i++) { pic->motion_val[i] = (int16_t (*)[2])pic->motion_val_buf[i]->data + 4; pic->ref_index[i] = pic->ref_index_buf[i]->data; + /* FIXME: The output of H.263 with OBMC depends upon + * the earlier content of the buffer; therefore we + * reset it here. */ + memset(pic->motion_val_buf[i]->data, 0, pic->motion_val_buf[i]->size); } } diff --git a/tests/ref/vsynth/vsynth1-h263-obmc b/tests/ref/vsynth/vsynth1-h263-obmc index b7a267a8cb..aed283ed53 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 +2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo +stddev: 8.38 PSNR: 29.66 MAXDIFF: 116 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth2-h263-obmc b/tests/ref/vsynth/vsynth2-h263-obmc index 2cef7f551b..c0dcc3239e 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 +3500b4227c1e6309ca5213414599266f *tests/data/fate/vsynth2-h263-obmc.out.rawvideo +stddev: 6.19 PSNR: 32.29 MAXDIFF: 111 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth_lena-h263-obmc b/tests/ref/vsynth/vsynth_lena-h263-obmc index 5b963107f6..78d7cc7277 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 +737af7fb166e2260ba049ae6bc30673d *tests/data/fate/vsynth_lena-h263-obmc.out.rawvideo +stddev: 5.42 PSNR: 33.44 MAXDIFF: 77 bytes: 7603200/ 7603200
Codecs call ff_find_unused_picture() to get the index of an unused picture; said picture may have buffers left from using it previously (these buffers are intentionally not unreferenced so that it might be possible to reuse them; this is mpegvideo's version of a bufferpool). They should not make any assumptions about which picture they get. Yet somehow this is not true when decoding OBMC: Returning random empty pictures (instead of the first one) leads to nondeterministic results; similarly, explicitly rezeroing the buffer before handing it over to the codec changes the outcome of the h263-obmc tests, but it makes it independent of the returned pictures. Therefore this commit does so. (No, this commit is not intended to be applied. I just hope to arouse the interest of people familiar with H.263 to look at this issue.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/mpegpicture.c | 4 ++++ 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, 10 insertions(+), 6 deletions(-)