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

[Michael Niedermayer]

On Mon, Jun 21, 2010 at 08:31:28PM +0100, M?ns Rullg?rd wrote:
> 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.

I dont understand your problem, nor can i relate your rant to the existing
code
The code as is, is fast, simple and there are no special cases in any idct
currently
You seem to start your argumentation on the need of a change.
I would prefer if you started with a description of what you plan to do
or a description of in which way the current system would be in your way

The coefficients an idct gets are the correct input coefficients
block_last_index is exactly what a optimized idct should check for
if such idct would also be enabled for mpeg2 then it will need to
support a [63]=1 case too, it will need to support that either way
or there would be no point in enabling it for mpeg2 because it would
not be useable for 50% of the blocks on which it otherwise would.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100622/5b43253b/attachment.pgp>