[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 07:12:44PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Mon, Jun 21, 2010 at 08:00:33PM +0200, Michael Niedermayer wrote:
>> >> On Mon, Jun 21, 2010 at 05:12:46PM +0100, M?ns Rullg?rd wrote:
>> >> > Michael Niedermayer <michaelni at gmx.at> writes:
>> >> > 
>> >> > >> Having an incorrect value in block_last_index just seems
>> >> > >> wrong to me.
>> >> > >
>> >> > > i dont think a completely correct value would be very
>> >> > > efficient speedwise as 75% of the cases where the last coeff
>> >> > > is set have no effect on the post dct output for dc blocks
>> >> > > (that is with an assumtation of dc values all being equally
>> >> > > likely)
>> >> > 
>> >> > The last coeff is always manipulated, not only for dc-only blocks.  Do
>> >> > you know for sure exactly in which cases it makes a difference?
>> >> 
>> >> I dont think anything except the idct is using that coeff but i 
>> >> i cannot formally proof that it is so or is bug free ...
>> >> If you have doubts, its surely possible to perform some tests to see
>> >> if its value makes a difference outside the idcts
>> >
>> > Also to everyone to whom things feel semantically wrong
>> > one can think of mismatch control to be part of the idct, which isnt
>> > far fetched semantically. In that case it makes sense not to update
>> > block_last_index. And we then would just be messing with he 64th coeff
>> > in the coeff decoder because thats faster there than seperately later.
>> 
>> Sure, but it's hard to optimise the idct properly when the input
>> parameters are wrong.  Choosing various shortcuts based on
>> block_last_index is a natural thing to do, but it's not possible when
>> this value cannot be trusted.
>
> That specific idct can be disabled or replaced by something else for mpeg2
>
> Setting block_last_index "correctly" just means setting it to 63
> that will just disable the optimisations

Then the "correction" applied to coeff 63 needs to be flagged by some
other means.  Although the coeff decoding is obviously the most
efficient place to calculate this adjustment, knowing the actual last
coded coeff is useful for the idct.  Relying on the idct compensating
for this munging is IMO totally out of the question.

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