[FFmpeg-devel,05/11] avcodec/mpegenc: do not use unquantize shortcuts for wmv1

Submitted by James Darnley on June 19, 2017, 3:10 p.m.

Details

Message ID 20170619151104.31273-6-jdarnley@obe.tv
State New
Headers show

Commit Message

James Darnley June 19, 2017, 3:10 p.m.
From: "Ronald S. Bultje" <rsbultje@gmail.com>

Commit message by James Darnley

The shortcut is based on end-of-block positions.  This leads to some
coefficients not being unquantized.  This is the symptom of the bug.

A possible candidate for the real bug is the scan table used here in
unquantize does not appear to match the one used in wmv1.  That might be
because h263_unquantize_intra uses the inter scan table.

We can avoid the bug by not using the shortcut.
---
 libavcodec/mpegvideo.c     | 2 +-
 libavcodec/x86/mpegvideo.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Michael Niedermayer June 19, 2017, 10:08 p.m.
On Mon, Jun 19, 2017 at 05:10:58PM +0200, James Darnley wrote:
> From: "Ronald S. Bultje" <rsbultje@gmail.com>
> 
> Commit message by James Darnley
> 
> The shortcut is based on end-of-block positions.  This leads to some
> coefficients not being unquantized.  This is the symptom of the bug.
> 

> A possible candidate for the real bug is the scan table used here in
> unquantize does not appear to match the one used in wmv1.  That might be
> because h263_unquantize_intra uses the inter scan table.

changed it to intra, i hope this makes this patch here unneeded

[..]
James Darnley June 20, 2017, 10:58 a.m.
On 2017-06-20 00:08, Michael Niedermayer wrote:
> On Mon, Jun 19, 2017 at 05:10:58PM +0200, James Darnley wrote:
>> From: "Ronald S. Bultje" <rsbultje@gmail.com>
>>
>> Commit message by James Darnley
>>
>> The shortcut is based on end-of-block positions.  This leads to some
>> coefficients not being unquantized.  This is the symptom of the bug.
>>
> 
>> A possible candidate for the real bug is the scan table used here in
>> unquantize does not appear to match the one used in wmv1.  That might be
>> because h263_unquantize_intra uses the inter scan table.
> 
> changed it to intra, i hope this makes this patch here unneeded

Yes it does appear to be unneeded.  Thank you.

Patch hide | download patch | download mbox

diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index e5424cbacf..568e782dd6 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -222,7 +222,7 @@  static void dct_unquantize_h263_intra_c(MpegEncContext *s,
     }else{
         qadd = 0;
     }
-    if(s->ac_pred)
+    if(s->ac_pred || s->avctx->codec_id == AV_CODEC_ID_WMV1)
         nCoeffs=63;
     else
         nCoeffs= s->inter_scantable.raster_end[ s->block_last_index[n] ];
diff --git a/libavcodec/x86/mpegvideo.c b/libavcodec/x86/mpegvideo.c
index 35a8264804..43df9a71b5 100644
--- a/libavcodec/x86/mpegvideo.c
+++ b/libavcodec/x86/mpegvideo.c
@@ -48,7 +48,7 @@  static void dct_unquantize_h263_intra_mmx(MpegEncContext *s,
         qadd = 0;
         level= block[0];
     }
-    if(s->ac_pred)
+    if(s->ac_pred || s->avctx->codec_id == AV_CODEC_ID_WMV1)
         nCoeffs=63;
     else
         nCoeffs= s->inter_scantable.raster_end[ s->block_last_index[n] ];