[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 02:19:22PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Mon, Jun 21, 2010 at 11:10:55AM +0100, M?ns Rullg?rd wrote:
>> >> Michael Niedermayer <michaelni at gmx.at> writes:
>> >> 
>> >> > On Sun, Jun 20, 2010 at 07:35:16PM -0700, Jason Garrett-Glaser wrote:
>> >> >> 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:
>> >> >> >> >> >> - ? ?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?
>> >> >
>> >> > for which dc value is it non bitexact?
>> >> 
>> >> RTFS, all even values.
>> >
>> > before the (reference) idct but not after it AFIAK
>> 
>> Please elaborate.
>
> Iam an idiot
> I thought the small contribution of the 64th coeff being 1 would
> make no difference unless something ended pretty close to 0.5 and
> i further thought that the 1 case didnt occur for such dc values.
> I even tested it quickly but messed up again, -2 vs. -3 ...
> i retested it now and as long as 
> dc % 8 != 4 the 64th coeff being 1 for even makes no difference
> for dc only blocks
>
> either way, a (block[63]&7) == 4 test before the dc idct seems
> better to me than updating last_index for all even dc values as
> that way the dc idct can be used for more cases

Having an incorrect value in block_last_index just seems wrong to me.

Also, this mismatch control thing is only used in mpeg2.  Extra checks
elsewhere would needlessly penalise other codecs.

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