Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10366 - Optional debug output for sound.c

Attached to Project: Rockbox
Opened by Jeffrey Goode (Blue_Dude) - Monday, 22 June 2009, 02:38 GMT+2
Last edited by Thomas Martitz (kugel.) - Monday, 22 June 2009, 20:51 GMT+2
Task Type Patches
Category Simulator
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Version 3.3
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Replaces debug output with optional log output. Output is enabled by uncommenting a define statement. Disabled by default.
   sound-logf.patch (1.7 KiB) 
 uisimulator/sdl/sound.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

This task depends upon

Closed by  Thomas Martitz (kugel.)
Monday, 22 June 2009, 20:51 GMT+2
Reason for closing:  Rejected
Additional comments about closing:  Committed the last patch targetting a cleanup in r21467.

We agreed on not needing (=rejecting) the initial patch though.
Comment by Dave Chapman (linuxstb) - Monday, 22 June 2009, 11:04 GMT+2
I tried to talk to you about this in IRC last night, but we missed each other. I asked why these patches don't simply replace DEBUGF with logf and you replied a couple of hours later saying " There's no problem with that, except it would be nice to turn off logf when not building for a simulator".

By default, logf _is_ turned off when building for a real target. You need to explicitly enable it (in the Advanced options in configure, or by #defining ROCKBOX_HAS_LOGF in autoconf.h).

It doesn't seem optimal that these patches (including the two that have been committed already) add yet another debugging macro (LOGFQUEUE).
Comment by Jeffrey Goode (Blue_Dude) - Monday, 22 June 2009, 14:42 GMT+2
This is true. But it's also true that it's easy to miss the enable define when building for a real target. The macro I put in is automatically disabled even if logf is mistakenly left active. OK, that's pretty weak, but when I saw the macro construction in another file it seemed like a good idea.

I'll modify the patches and post them here.
Comment by Jeffrey Goode (Blue_Dude) - Monday, 22 June 2009, 15:10 GMT+2
This should fix all of them.
   logf.patch (3.1 KiB) 
 apps/recorder/albumart.c |   10 ++--------
 apps/metadata/vorbis.c   |    8 +-------
 uisimulator/sdl/sound.c  |   13 ++++++++-----
 3 files changed, 11 insertions(+), 20 deletions(-)

Comment by Jeffrey Goode (Blue_Dude) - Monday, 22 June 2009, 19:08 GMT+2
Upon further discussion, sound.c doesn't need patching, but the other two need cleanup. Here's the cleanup.
   logf.patch (1.5 KiB) 
 apps/recorder/albumart.c |   10 ++--------
 apps/metadata/vorbis.c   |    8 +-------
 2 files changed, 3 insertions(+), 15 deletions(-)

Loading...