[Ffmpeg-cvslog] r6813 - trunk/libavcodec/eval.c

[Oded Shimon]

On Sat, Oct 28, 2006 at 05:07:08PM +0200, Michael Niedermayer wrote:
> Hi
> 
> On Sat, Oct 28, 2006 at 11:39:56AM +0200, Oded Shimon wrote:
> > On Sat, Oct 28, 2006 at 10:36:33AM +0100, M?ns Rullg?rd wrote:
> > > Oded Shimon <ods15 at ods15.dyndns.org> writes:
> > > 
> > > > On Sat, Oct 28, 2006 at 08:46:13AM +0200, Oded Shimon wrote:
> > > >> On Sat, Oct 28, 2006 at 12:16:26AM +0200, michael wrote:
> > > >> [..]
> > > >> >                  case e_div: return e->value * (d / d2);
> > > >> >                  case e_add: return e->value * (d + d2);
> > > >> > +                case e_last:return d2;
> > > >> 
> > > >> I'm fairly sure there should be an 'e->value *' here... try the expression 
> > > >> '5;-2'
> > > >
> > > > I was almost right, '-(1;2)'
> > > 
> > > What does that mean?
> > 
> > ';' is a statement seperator, the first value is ignored, the second is 
> > used as the result of the expression. So, '(1;2)' returns simply '2'. The 
> > bug was that the '-' was ignored, and the overall expression returned '2' 
> > instead of '-2'. The reason for that is that the sign is stored always in 
> > e->value.
> 
> btw, what is your oppinion about getting rid of thet value* hack? and replace
> it either by a negate(x) operation or pass the sign up and switch add to sub 
> (with neg() only if that isnt possible) ?
> ive implemented both but they are a few lines of code more so iam not sure
> if its a good idea or not

No real thoughts, just make sure to not make a redundant negate on a const 
value. (for e_value, just multiply by the sign..)

- ods15