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

[Måns Rullgård]

Jason Garrett-Glaser <darkshikari at gmail.com> writes:

> On Sun, Jun 20, 2010 at 7:00 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> On Mon, Jun 21, 2010 at 02:47:16AM +0100, M?ns Rullg?rd wrote:
>>> 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.
>>
>> you are unable to ignore block[63] in the dc idct ?
>
> And have non-bit-exact decoding?

It wouldn't even be spec-compliant since the "mismatch control" would
be skipped for otherwise dc-only blocks.

-- 
M?ns Rullg?rd
mans at mansr.com