[Ffmpeg-devel] xprintf cleanup: wma.c
Tue Feb 20 16:00:13 CET 2007
Michael Niedermayer wrote:
> 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
> 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.
T +32  2 790 29 41
F +32  2 790 29 02
E mailto:mbardiaux at mediaxim.be
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
More information about the ffmpeg-devel