[FFmpeg-devel] [PATCH][RFC] Lagarith Decoder.

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:

> On Thu, Sep 17, 2009 at 04:36:45PM +0100, M?ns Rullg?rd wrote:
>> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
>> 
>> > On Thu, Sep 17, 2009 at 02:46:07AM -0600, Nathan Caldwell wrote:
>> >> 2009/9/5 Loren Merritt <lorenm at u.washington.edu>:
>> >> > On Tue, 1 Sep 2009, Nathan Caldwell wrote:
>> >> >
>> >> >> Here are the latest lagarith patches.
>> >> >
>> >> > Your decoder disagrees with the official version on the 2nd frame of http://akuvian.org/stuff/lagarith_bork.avi
>> >> 
>> >> Finally got a chance to look into this, and found the problem, but I'm
>> >> not sure how to fix it.
>> >> What happens is on this clip when the probabilities get scaled in
>> >> lag_read_prob_header(), one of the values gets rounded up instead of
>> >> rounding down like reference does. For the offending value reference
>> >> gives 0x6ff while the code below gives 0x700.
>> >
>> > Besides a "told you so" as to using floating point giving no end of
>> > issues for lossless, the exact input values would be the minimum
>> > necessary to help out.
>> > Adding a volatile float or volatile double at the right place might
>> > be all that is necessary to make it work (well as far as it can be
>> > called "working" while using floating point).
>> 
>> WTF?  How would adding volatile change anything whatsoever?
>
> It can force rounding to IEEE precision of intermediate results, or a
> specific evaluation order.

Can != does.  This is not required or even documented behaviour.  We
should not rely on it.

>> If it does, it's an accidental side-effect and NOT THE RIGHT
>> SOLUTION.  98% of all "solutions" involving volatile are wrong.
>
> Well, I didn't call it a solution, I said "make it work". If you are
> using floating point for such code, using volatile is not going to make
> it any worse.

It is likely to make the code slower, but maybe that doesn't matter.
I think the proper solution is something that avoids floating-point
entirely.

> As to those asking how the reference code manages: Probably by relying
> on luck that the encoder and the decoder will happen to use the same
> instructions and thus the same rounding errors.
> I'd expect it to be the kind of code that fails when e.g.
> you try to upgrade your compiler or use ICL instead of VC compiler and
> things like that (i.e. "I managed to write C code that is less portable
> than plain asm code"). Of course I'd be happy to be wrong...

I have no doubt this is how it happened.

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