[FFmpeg-cvslog] r14267 - trunk/libavcodec/ra288.c

Vitor Sessak vitor1001
Sat Jul 19 14:57:46 CEST 2008


Reimar D?ffinger wrote:
> Hello,
> On Fri, Jul 18, 2008 at 11:41:20PM +0200, Vitor Sessak wrote:
>> Reimar D?ffinger wrote:
>>> On Fri, Jul 18, 2008 at 12:35:55PM +0200, Vitor Sessak wrote:
>>>> Diego Biurrun wrote:
>>>>> - at least mention which function you simplified.
>>>> That I'm not against. Are you ok with something like "Minor/Major 
>>>> simplification in decode()"?
>>> Well, it is better, but obviously that works best when the function
>>> names are not completely worthless. In this case, where the function
>>> has such a completely useless name as "pred" (and no, it is not just
>>> a 3-line function) it obviously does not help much.
>> If ra288.c wasn't an ugly mess, I wouldn't spend my time cleaning it up ;-)
> 
> Just to  make it clear: I am well aware of your efforts and appreciate
> it. And also, e.g. what I would have considered a better (and in this
> case possibly best possible) commit message would be something like
> (assuming it is not factually wrong, I did not check in detail what your
> change does): "simplify ra288 predictor calculation".

Yes, this would be a nice commit message. But for doing cleanup, I 
follow roughly the following steps:

1- Simplify obviously obfuscated C code
2- RTFSpec and try to give decent names for functions and simplify more

I'm still at 1 and it looks that pred() is the "backward prediction 
adaptation", but I'm not really sure yet, so writing "predictor" in the 
log is risky...

> And while I do think that including the file name(s) just out of
> principle makes no sense, I do think that a good log message usually
> will end up mentioning it. A good log message is one that without
> additional information gives me an idea of how the actual patch looks
> like and would make it much simpler to recreate the patch than it would
> be without reading the log message.
> 
>> PS: if you want to see an even worse mess, looks at ra144.c at revision 
>> 13009...
> 
> Thanks, but not thanks. I had enough of mess in MPlayer and I probably
> can still find a lot more there if I really want...

It is not the first time I heard of very messy code in MPlayer. I'm 
starting to get scared... ;-)

-Vitor




More information about the ffmpeg-cvslog mailing list