[FFmpeg-devel] [PATCH] mpeg2: fix block_last_index when mismatch control modifies last coeff

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Mon, Jun 21, 2010 at 12:40:14AM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Sun, Jun 20, 2010 at 11:41:32PM +0100, Mans Rullgard wrote:
>> >> ---
>> >>  libavcodec/mpeg12.c |    4 ++--
>> >>  1 files changed, 2 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
>> >> index bc9ddcc..bd579ac 100644
>> >> --- a/libavcodec/mpeg12.c
>> >> +++ b/libavcodec/mpeg12.c
>> >> @@ -939,7 +939,7 @@ end:
>> >>      }
>> >>      block[63] ^= (mismatch & 1);
>> >>  
>> >> -    s->block_last_index[n] = i;
>> >> +    s->block_last_index[n] = block[63]? 63: i;
>> >>      return 0;
>> >>  }
>> >>  
>> >> @@ -1090,7 +1090,7 @@ static inline int mpeg2_decode_block_intra(MpegEncContext *s,
>> >>      }
>> >>      block[63]^= mismatch&1;
>> >>  
>> >> -    s->block_last_index[n] = i;
>> >> +    s->block_last_index[n] = block[63]? 63: i;
>> >>      return 0;
>> >
>> > is this fixing a bug?
>> 
>> The value of block_last_index is wrong otherwise.  We're trying to add
>> a dc-only idct (and perhaps other long zero tails).  That requires a
>> correct block_last_index value.
>> 
>> > because, if it makes no difference, then it would cause a speedloss
>> > and possibly not small if this is a conditional branch that fails to be
>> > predicted
>> 
>> Do you have a better suggestion?
>
> ignore it?

Then the optimisation is impossible.  How about this patch?

diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index bc9ddcc..aad6670 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -939,7 +939,8 @@ end:
     }
     block[63] ^= (mismatch & 1);
 
-    s->block_last_index[n] = i;
+    j = block[63] & 1;
+    s->block_last_index[n] = i | ((j << 6) - j);
     return 0;
 }
 
@@ -1090,7 +1091,8 @@ static inline int mpeg2_decode_block_intra(MpegEncContext *s,
     }
     block[63]^= mismatch&1;
 
-    s->block_last_index[n] = i;
+    j = block[63] & 1;
+    s->block_last_index[n] = i | ((j << 6) - j);
     return 0;
 }
 
-- 
M?ns Rullg?rd
mans at mansr.com