[PATCH] if(verbose>X) --> mp_msg_test
Hi, I was staring at progress bars a lot at work yesterday, so I did some janitorial work that should help the pending mp_msg patch from attila. I've converted all the 'if (verbose > X)' calls to mp_msg_test, which is the proper way and gets us rid of a few global vars as an added bonus. I intend to apply this tomorrow, shout now if I messed up somewhere. Diego
On Wed, Mar 22, 2006 at 12:53:37AM +0100, Diego Biurrun wrote:
Hi,
I was staring at progress bars a lot at work yesterday, so I did some janitorial work that should help the pending mp_msg patch from attila.
I've converted all the 'if (verbose > X)' calls to mp_msg_test, which is the proper way and gets us rid of a few global vars as an added bonus.
I intend to apply this tomorrow, shout now if I messed up somewhere.
Diego
Hi Diego -if (verbose>1) print_wave_header(mux_a->wf); +if ( mp_msg_test(MSGT_MENCODER,MSGL_V) ) print_wave_header(mux_a->wf, MSGL_V); Shouldnt this one be: mp_msg_test(MSGT_MENCODER,MSGL_DBG2) ? I mean: if(verbose>0) == mp_msg_test(MSGT_MENCODER,MSGL_V) Same here but its commented out so who cares +// if(mp_msg_test(MSGT_CPLAYER,MSGL_V)printf("sleep: %5.3f a:%6.3f v:%6.3f \n",time_frame,sh_audio->timer,sh_video->timer); Havent checked te rest of it yet. Best regards Reynaldo
On Tue, Mar 21, 2006 at 08:26:21PM -0400, Reynaldo H. Verdejo Pinochet wrote:
On Wed, Mar 22, 2006 at 12:53:37AM +0100, Diego Biurrun wrote:
I was staring at progress bars a lot at work yesterday, so I did some janitorial work that should help the pending mp_msg patch from attila.
I've converted all the 'if (verbose > X)' calls to mp_msg_test, which is the proper way and gets us rid of a few global vars as an added bonus.
I intend to apply this tomorrow, shout now if I messed up somewhere.
-if (verbose>1) print_wave_header(mux_a->wf); +if ( mp_msg_test(MSGT_MENCODER,MSGL_V) ) print_wave_header(mux_a->wf, MSGL_V);
Shouldnt this one be: mp_msg_test(MSGT_MENCODER,MSGL_DBG2) ?
I mean:
if(verbose>0) == mp_msg_test(MSGT_MENCODER,MSGL_V)
Same here but its commented out so who cares
+// if(mp_msg_test(MSGT_CPLAYER,MSGL_V)printf("sleep: %5.3f a:%6.3f v:%6.3f \n",time_frame,sh_audio->timer,sh_video->timer);
Havent checked te rest of it yet.
Fixed locally, thanks. Diego
On Wed, 22 Mar 2006 00:53:37 +0100 Diego Biurrun <diego@biurrun.de> wrote:
--- libmpdemux/aviheader.c 9 Feb 2006 00:40:35 -0000 1.70 +++ libmpdemux/aviheader.c 20 Mar 2006 22:07:47 -0000
[ ...]
@@ -202,7 +202,7 @@ memcpy(&sh_audio->audio,&h,sizeof(h)); } last_fccType=h.fccType; - if(verbose>=1) print_strh(&h); + if( mp_msg_test(MSGT_HEADER) ) print_strh(&h); break; } case mmioFOURCC('i', 'n', 'd', 'x'): {
One argument is missing here. Albeu
On Wed, Mar 22, 2006 at 02:38:09AM +0100, Alban Bedel wrote:
On Wed, 22 Mar 2006 00:53:37 +0100 Diego Biurrun <diego@biurrun.de> wrote:
--- libmpdemux/aviheader.c 9 Feb 2006 00:40:35 -0000 1.70 +++ libmpdemux/aviheader.c 20 Mar 2006 22:07:47 -0000
[ ...]
@@ -202,7 +202,7 @@ memcpy(&sh_audio->audio,&h,sizeof(h)); } last_fccType=h.fccType; - if(verbose>=1) print_strh(&h); + if( mp_msg_test(MSGT_HEADER) ) print_strh(&h); break; } case mmioFOURCC('i', 'n', 'd', 'x'): {
One argument is missing here.
Fixed locally, thanks. Diego
On Wed, Mar 22, 2006 at 12:53:37AM +0100, Diego Biurrun wrote:
I've converted the proper way and gets us rid of a few global vars as an added bonus.
Forgot to mention: There are some 'if (verbose > 4)' calls, for which I would have to add something like the following: --- mp_msg.h 9 Feb 2006 14:07:52 -0000 1.37 +++ mp_msg.h 23 Mar 2006 09:36:14 -0000 @@ -20,6 +20,7 @@ #define MSGL_DBG2 7 // v=2 #define MSGL_DBG3 8 // v=3 #define MSGL_DBG4 9 // v=4 +#define MSGL_DBG5 10 // v=5 Anything I overlooked? Otherwise I will doublecheck and commit tonight or so. Diego
On Thu, Mar 23, 2006 at 10:38:00AM +0100, Diego Biurrun wrote:
On Wed, Mar 22, 2006 at 12:53:37AM +0100, Diego Biurrun wrote:
I've converted the proper way and gets us rid of a few global vars as an added bonus.
Forgot to mention: There are some 'if (verbose > 4)' calls, for which I would have to add something like the following:
--- mp_msg.h 9 Feb 2006 14:07:52 -0000 1.37 +++ mp_msg.h 23 Mar 2006 09:36:14 -0000 @@ -20,6 +20,7 @@ #define MSGL_DBG2 7 // v=2 #define MSGL_DBG3 8 // v=3 #define MSGL_DBG4 9 // v=4 +#define MSGL_DBG5 10 // v=5
Seems ok but I wonder if we really need so many DBG levels, are there any rules on how to decide wich DBGX to use? if there arent this may be the right time to make some.
Anything I overlooked? Otherwise I will doublecheck and commit tonight or so.
Looks ok.
Diego
Best regards Reynaldo
On Thu, Mar 23, 2006 at 01:22:09PM -0400, Reynaldo H. Verdejo Pinochet wrote:
On Thu, Mar 23, 2006 at 10:38:00AM +0100, Diego Biurrun wrote:
Anything I overlooked? Otherwise I will doublecheck and commit tonight or so.
Looks ok.
Patch applied. BTW, there are 3 calls to if(verbose >0) left in libmpdemux/demux_rtp.cpp. I'm not sure if #including mp_msg.h from a C++ file might make any troubles so I've left them unchanged for the moment. Diego
2006. március 24. 09.13 dátummal Diego Biurrun ezt írta: Hi Diego!
Patch applied.
Shall I update my printf patch? Best regards! Attila
2006. március 24. 20.18 dátummal Diego Biurrun ezt írta:
On Fri, Mar 24, 2006 at 08:11:38PM +0100, Ötvös Attila wrote:
2006. március 24. 09.13 dátummal Diego Biurrun ezt írta: Hi Diego!
Patch applied.
Shall I update my printf patch?
Yes, please. I updated.
Best regards. Attila
On Sun, Mar 26, 2006 at 11:41:21AM +0200, Ötvös Attila wrote:
2006. március 24. 20.18 dátummal Diego Biurrun ezt írta:
On Fri, Mar 24, 2006 at 08:11:38PM +0100, Ötvös Attila wrote:
Shall I update my printf patch?
Yes, please. I updated.
At a first glance: Please structure your additions to help_mp-en.h a bit, i.e. add comments describing to which files the messages belong like it's done in the rest of the file. Diego
On Sun, Mar 26, 2006 at 11:43:40AM +0200, Diego Biurrun wrote:
On Sun, Mar 26, 2006 at 11:41:21AM +0200, Ötvös Attila wrote:
2006. március 24. 20.18 dátummal Diego Biurrun ezt írta:
On Fri, Mar 24, 2006 at 08:11:38PM +0100, Ötvös Attila wrote:
Shall I update my printf patch?
Yes, please. I updated.
At a first glance: Please structure your additions to help_mp-en.h a bit, i.e. add comments describing to which files the messages belong like it's done in the rest of the file.
At a second glance: MSGL_V and above messages need not be translated as they are only debug output intended for developers, so they should not be moved to help_mp-en.h. Diego
2006. március 26. 11.47 dátummal Diego Biurrun ezt írta:
At a first glance: Please structure your additions to help_mp-en.h a bit, i.e. add comments describing to which files the messages belong like it's done in the rest of the file.
At a second glance: MSGL_V and above messages need not be translated as they are only debug output intended for developers, so they should not be moved to help_mp-en.h.
Add #include "help_mp.h" Remove MSGTR_LIBVO_SdlMode Add c file name to help_mp-en.h in comment. Best regards! Attila
On Sun, Mar 26, 2006 at 10:23:46PM +0200, Ötvös Attila wrote:
2006. március 26. 11.47 dátummal Diego Biurrun ezt írta:
At a first glance: Please structure your additions to help_mp-en.h a bit, i.e. add comments describing to which files the messages belong like it's done in the rest of the file.
At a second glance: MSGL_V and above messages need not be translated as they are only debug output intended for developers, so they should not be moved to help_mp-en.h.
Add #include "help_mp.h" Remove MSGTR_LIBVO_SdlMode Add c file name to help_mp-en.h in comment.
I've ripped out and applied the printf --> fprintf part to get things started. liba52 should not be patched, it's an imported library, there is still one if (verbose) in vf_tile.c that you removed. I've fixed the remaining if(verbose) calls. Other than that the patch looks OK at a quick glance. It should probably be applied piece by piece to make review easier unless somebody jumps in and reviews it as a whole. Diego
On Mon, Mar 27, 2006 at 10:00:45AM +0200, Diego Biurrun wrote:
I've ripped out and applied the printf --> fprintf part to get things started.
liba52 should not be patched, it's an imported library, there is still one if (verbose) in vf_tile.c that you removed. I've fixed the remaining if(verbose) calls.
Other than that the patch looks OK at a quick glance. It should probably be applied piece by piece to make review easier unless somebody jumps in and reviews it as a whole.
Reynaldo, can you look into this? Probably it's easiest to split the patch in pieces and review/apply them separately, it's quite large. Diego
On Wed, Mar 29, 2006 at 01:27:00PM +0200, Diego Biurrun wrote:
On Mon, Mar 27, 2006 at 10:00:45AM +0200, Diego Biurrun wrote:
I've ripped out and applied the printf --> fprintf part to get things started.
liba52 should not be patched, it's an imported library, there is still one if (verbose) in vf_tile.c that you removed. I've fixed the remaining if(verbose) calls.
Other than that the patch looks OK at a quick glance. It should probably be applied piece by piece to make review easier unless somebody jumps in and reviews it as a whole.
Reynaldo, can you look into this? Probably it's easiest to split the patch in pieces and review/apply them separately, it's quite large.
Diego
No prob, sorry for the delay. Reynaldo
On Wed, Mar 29, 2006 at 11:53:59PM -0400, Reynaldo H. Verdejo Pinochet wrote:
On Wed, Mar 29, 2006 at 01:27:00PM +0200, Diego Biurrun wrote:
On Mon, Mar 27, 2006 at 10:00:45AM +0200, Diego Biurrun wrote:
I've ripped out and applied the printf --> fprintf part to get things started.
liba52 should not be patched, it's an imported library, there is still one if (verbose) in vf_tile.c that you removed. I've fixed the remaining if(verbose) calls.
Other than that the patch looks OK at a quick glance. It should probably be applied piece by piece to make review easier unless somebody jumps in and reviews it as a whole.
Reynaldo, can you look into this? Probably it's easiest to split the patch in pieces and review/apply them separately, it's quite large.
Diego
No prob, sorry for the delay.
Reynaldo
Ok Im reviewing fixing and commiting his changes in small parts, Im almost sure they could still have gram and sintax errors but imho they can be easily fixed once in the codebase. I think this is the right aproach, hope is ok. Reynaldo
_______________________________________________ MPlayer-dev-eng mailing list MPlayer-dev-eng@mplayerhq.hu http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
On Sun, Mar 26, 2006 at 10:23:46PM +0200, Ötvös Attila wrote: liba52 should not be patched, it's an imported library, there is still one if (verbose) in vf_tile.c that you removed. I've fixed the remaining if(verbose) calls.
2006. március 27. 10.00 dátummal Diego Biurrun ezt írta: liba52 removed vf_tile modify
Other than that the patch looks OK at a quick glance. It should probably be applied piece by piece to make review easier unless somebody jumps in and reviews it as a whole.
I separate patch to files and updated with CVS. Best regards! Attila
On Wed, Mar 29, 2006 at 07:08:05PM +0200, Ötvös Attila wrote:
On Sun, Mar 26, 2006 at 10:23:46PM +0200, Ötvös Attila wrote: liba52 should not be patched, it's an imported library, there is still one if (verbose) in vf_tile.c that you removed. I've fixed the remaining if(verbose) calls.
2006. március 27. 10.00 dátummal Diego Biurrun ezt írta: liba52 removed vf_tile modify
Other than that the patch looks OK at a quick glance. It should probably be applied piece by piece to make review easier unless somebody jumps in and reviews it as a whole.
I separate patch to files and updated with CVS.
Ok Otvos, Im commiting your changes with minor corrections, just a quick advice Im sure your future reviewers will appreaciate: when facing the need of spliting a big patch into smaller units, the important thing is to make sure each unit can be applied separately, there is no point of making 30 patches that can only be applyed all together. Thanks again for your time and work. Best regards Reynaldo
2006. március 30. 08.46 dátummal Reynaldo H. Verdejo Pinochet ezt írta:
Ok Otvos, Im commiting your changes with minor corrections, just a quick advice Im sure your future reviewers will appreaciate:
when facing the need of spliting a big patch into smaller units, the important thing is to make sure each unit can be applied separately, there is no point of making 30 patches that can only be applyed all together.
Thanks again for your time and work.
Thank you! Best regards! Attila
On Fri, Mar 31, 2006 at 07:57:02PM +0200, Ötvös Attila wrote:
2006. március 30. 08.46 dátummal Reynaldo H. Verdejo Pinochet ezt írta:
Ok Otvos, Im commiting your changes with minor corrections, just a quick advice Im sure your future reviewers will appreaciate:
when facing the need of spliting a big patch into smaller units, the important thing is to make sure each unit can be applied separately, there is no point of making 30 patches that can only be applyed all together.
Thanks again for your time and work.
Thank you!
[wrong thread] Commited. Reynaldo
On Sun, Mar 26, 2006 at 11:47:07AM +0200, Diego Biurrun wrote:
On Sun, Mar 26, 2006 at 11:43:40AM +0200, Diego Biurrun wrote:
On Sun, Mar 26, 2006 at 11:41:21AM +0200, Ötvös Attila wrote:
2006. március 24. 20.18 dátummal Diego Biurrun ezt írta:
On Fri, Mar 24, 2006 at 08:11:38PM +0100, Ötvös Attila wrote:
Shall I update my printf patch?
Yes, please. I updated.
At a first glance: Please structure your additions to help_mp-en.h a bit, i.e. add comments describing to which files the messages belong like it's done in the rest of the file.
At a second glance: MSGL_V and above messages need not be translated as they are only debug output intended for developers, so they should not be moved to help_mp-en.h.
Diego
Sad I didnt noticed it earlier :( may the comment on mp_msg.h be more clear about this? Reynaldo
participants (4)
-
Alban Bedel -
Diego Biurrun -
Reynaldo H. Verdejo Pinochet -
Ötvös Attila