[MPlayer-dev-eng] Re: [PATCH] printf -> mp_msg

Rich Felker dalias at aerifal.cx
Mon Mar 20 16:03:29 CET 2006


On Mon, Mar 20, 2006 at 02:41:52PM +0100, Alban Bedel wrote:
> On Sun, 19 Mar 2006 23:35:36 -0500
> Rich Felker <dalias at aerifal.cx> wrote:
> 
> > On Sun, Mar 19, 2006 at 06:54:27PM -0400, Reynaldo H. Verdejo Pinochet wrote:
> > > On Sun, Mar 19, 2006 at 11:34:00PM +0100, Diego Biurrun wrote:
> > > > On Sun, Mar 19, 2006 at 07:24:37PM +0100, Ötvös Attila wrote:
> > > > > 
> > > > > liba52: 
> > > > > parse.c, ao_oss.c, ao_sun.c, audio_out.c
> > > > 
> > > > This is an imported library, don't patch it.
> > > > 
> > > > You have a few hunks like the following in the patch:
> > > > 
> > > > -    if(verbose > 4)
> > > > -        printf("vd_ffmpeg::mc_get_buffer\n");
> > > > +//    if(verbose > 4)
> > > > +        mp_msg(MSGT_DECVIDEO, MSGL_DBG4, "vd_ffmpeg::mc_get_buffer\n");
> > > > 
> > > > Somebody correct me if I am wrong, but I believe this is incorrect,
> > > > since now the call to mp_msg will not be skipped as it should.
> > > > 
> > > > Diego
> > > 
> > > I prefer the later, having if(verbose >|<|= X) clauses _and_ MSGL_X
> > > is just error prone, duplicated work and useles code polution IMHO.
> > 
> > No, the few cases where this is done it's critical -- at least for the
> > few I know. The performance penalty of calling mp_msg over and over in
> > these instances was deemed unacceptable so it was put under
> > conditional.
> 
> In case you don't know about it, in such cases you can also use the
> mp_dbg() macro. It expand to mp_msg only in debug builds.

Debug build is very different from verbose==4 and is not the intended
semantics.

Rich




More information about the MPlayer-dev-eng mailing list