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

Rich Felker dalias at aerifal.cx
Mon Mar 20 07:17:22 CET 2006


On Mon, Mar 20, 2006 at 06:37:46AM +0200, Oded Shimon wrote:
> On Sun, Mar 19, 2006 at 11:35:36PM -0500, Rich Felker 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.
> 
> These should be changed to mp_msg_test, and, there's no gain if the if is 
> put before a single mp_msg, it is only worth it if the if is preventing a 
> whole loop of it. (mp_msg returns immediately if message is not going to be 
> printed, no formatting or anything)

However this is totally not the case if something evil like gettext is
used, and there's even some (much smaller, O(1)) price to pay even
with catgets. Even without, a function call and conditional costs more
than a conditional alone.

Rich




More information about the MPlayer-dev-eng mailing list