[MPlayer-dev-eng] [PATCH] OpenDML AVI2.0 read support

The Wanderer inverseparadox at comcast.net
Fri Feb 6 23:51:14 CET 2004


D Richard Felker III wrote:

> On Fri, Feb 06, 2004 at 04:26:46PM -0500, The Wanderer wrote:
> 
>> Attila Kinali wrote:

>>> Please change the printfs to mp_msg calls.
>> 
>> Apropos of this: as of this morning's CVS, there are (depending on
>> which way you count it - I haven't been able to put together a
>> reliable regular expression which doesn't return false positives)
>> between several hundred and a few thousand printf() calls under the
>> MPlayer source tree, not counting any from libavcodec.

I've improved on this - there appear to be 3617 of them.

>> Given stated policy, I'd like to ask if there is any reason - other
>> than "no one noticed enough to point it out" - why these made it
>> in without being changed to mp_msg() or similar calls? For that
>> matter, would there be any problem with changing them, if someone
>> wanted to do the work of tracking them all down?
> 
> They should all be changed. The only reason they haven't is laziness.
> BTW, when changing them one needs to decide which message level to
> put them at, not just blindly search-and-replace.

Well enough, then. I'd be willing to volunteer for at least some of the
work, though I'd need to read up on the various message levels and what
belongs in each...

Should the printf() calls be changed in things like libavcodec,
libfaad2, mp3lib and so forth? I'd be reluctant to do it with
libavcodec, because I update that from its own CVS almost daily so the
changes would be lost, but with ostensibly-external things which are
included in MPlayer CVS directly the question becomes less obvious.
(Plainly, however, the calls in mp_msg.[ch] itself should be left
alone.)

> If you do blind s&r now, it would work ok, but then later on when
> someone wants to fix the message levels, it'll be difficult to figure
> out which messages already have intentionally-assigned levels and
> which don't.

Adam Rice's suggestion of defining FIXME_MESSAGE_LEVEL seems like a
workable solution to this; even if whoever changes the original printf()
doesn't intend to do blind search-and-replace, it could be useful if he
or she (or me) gets lazy.

-- 
       The Wanderer

A government exists to serve its citizens, not to control them.




More information about the MPlayer-dev-eng mailing list