[FFmpeg-devel] DVCPRO HD: request for review

Roman V. Shaposhnik rvs
Fri Aug 29 22:55:36 CEST 2008


On Fri, 2008-08-29 at 22:11 +0200, Michael Niedermayer wrote:
> well, some naive formular manipulation:
> if(X >= 80) {
>     if(Y<56) {
>         X+= tab1[Y];
>         Y = tab0[Y];
>     }else{
>         X = ((Y-64)*10 + X)*2;
>         Y = 67;
>     }
> }else
>     Y += 4;
> 
> 
> [...]

And the point of that would be? Well, I guess the only point I see
is that it is not as trivial as introducing a clever formula. You
had to introduce tab1 and tab0. Now it is not a question of the
old code being irregular and ugly and the new being all nice
and shiny -- it is a question of compromise. Can you do away
with tab1 & tab0 so that you don't have to spend time looking
things up in memory?

> > P.S. Besides you, yourself, have repeatedly indicated that you
> > do accept temporary commits of suboptimal code provided that
> > a replacement is committed really quickly. Heck! You even
> > admitted you do it yourself. So, why can't I? Especially
> > in a piece of code that I still maintain?
> 
> You can but there really is more than just the tables, the code did not
> pass review with some final list of whats left to make it clean. It seems
> you misunderstood me ok-ing some parts and not commenting others, that
> really wasnt supposed to mean the others are ok ...

My only comment then would be -- we have to have timeouts. As reasonable
developers we all know truly ugly code when we see it. It doesn't
take time to respond with: "its ugly, it needs to be speeded up, and
no I don't want to think about how exactly". I submitted my original
patch on Aug 17. Since then I gave you at least 3 extra chances to
comment on *everything* in the patch. And after 2 weeks it is only
now that you've found it possible to comment on the issues like 
the for loop in the previous email (which I assumed to be implicitly
ok'ed).

I'm fine with a default of a week or so to give you a chance of
identifying true deal breakers. If it takes more that a week
for you to see whether the code is good or bad, then by 
definition the code is OKish enough to be temporarily put
in SVN, provided that when you DO see an issue with it later
on I guarantee my fast response.

Of course, I'm only talking about the bits that you don't
officially maintain. And the normal flow of your life
(if there's an emergency -- all bets are off).

Does it seem fair to you?

Thanks,
Roman.





More information about the ffmpeg-devel mailing list