[Ffmpeg-devel] xprintf cleanup: wma.c

[Michel Bardiaux]

Michael Niedermayer wrote:
> Hi
> 
> On Mon, Feb 19, 2007 at 02:28:20PM +0100, Michel Bardiaux wrote:
>> It contains 1 first dprintf
>>
>>     dprintf("flags1=0x%x flags2=0x%x\n", flags1, flags2);
>>
>> that is probably a relic since there are no such variables visible.
> 
> if there are no such vars remove the dprintf
> 
> 
>> The following dprintf can easily become av_log.
>>
>> The 2nd #if around the tprintf can go away since tprintf is a nop anyway 
>> when TRACE is not defined.
> 
> theres a for loop in it ...

I would expect a good compiler to collapse all that to simply giving the 
loop index its max value. I may overestimate gcc's capabilities...

> 
> 
>> The 1st one is an #if 0 hence a question mark: keep or zorch?
> 
> not my code, and fabrice wont awnser (iam 99.99% sure about that ...)

So am I. But there must be a lot of code like that in ffmpeg (I mean, 
still as Fabrice wrote it), if there is no policy...

No big deal anyway, I'll just change the if0 to ifdef-TRACE.

> 
> 
>> tprintf itself is also a question. It uses av_log(NULL, which is bad. 
>> One coulddo one of the following:
>>
>> * Change all tprintf to av_log(VERBOSE) (or DEBUG)
>>
>> * Add a context argument to tprintf, but keep it a nop when TRACE not 
>> defined.
> 
> i like giving trpintf a context ...
> 
I agree. Now, I would rather not have to change all the calls at the 
same time (and you probably prefer not to have to review such a monster 
change!), so I propose adding a tprintfext function, then progressively 
removing all the tprintf.

HaND,
-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/