Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12240 - rbcodec refactoring part 1

Attached to Project: Rockbox
Opened by Sean Bartell (wtachi) - Monday, 22 August 2011, 08:42 GMT+2
Task Type Patches
Category Codecs
Status Unconfirmed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Private No

Details

Continuing from  FS#12204 , these patches remove some of the dependencies librbcodec has on the rest of Rockbox.

0001: puts equalizer settings in the parameters of dsp_set_eq_coefs, removing the need to access global_settings from dsp.c

0002: makes dsp_process yield each iteration instead of once each tick, which is tricky to do portably. If this is undesirable, I can work around it.

0003: moves some replaygain stuff around so dsp.c doesn't have to access global_settings.

0004: instead of dsp.c and tdspeed.c allocating buffers, buffers are allocated elsewhere and passed to dsp_timestretch_enable(). The necessary buffer size is determined by calling dsp_timestretch_get_buffer_size().

0005: replace get_audio_base_data_type() with audio_format_is_atomic(), removing a dependency on TYPE_PACKET_*.

0006: move autoresumable() to playback.c, removing another dependency on global_settings.
   0001-rbcodec-refactoring-dsp_set_eq_coefs.patch (16.1 KiB) 
 b/apps/menus/eq_menu.c  |   65 ++++++++++++++++++++++++------------------------
 b/apps/settings.c       |    4 ++
 b/apps/settings.h       |   31 ++--------------------
 b/apps/settings_list.c  |   30 +++++++++++-----------
 b/lib/rbcodec/dsp/dsp.c |   14 ++--------
 b/lib/rbcodec/dsp/dsp.h |    3 +-
 6 files changed, 59 insertions(+), 88 deletions(-)

   0002-rbcodec-refactoring-make-DSP-yield-each-iter... (1.5 KiB) 
 b/lib/rbcodec/dsp/dsp.c |   11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

   0003-rbcodec-refactoring-replaygain.patch (9.5 KiB) 
 b/apps/gui/skin_engine/skin_tokens.c |   28 ++++++--
 b/apps/menus/playback_menu.c         |    1 
 b/apps/misc.c                        |   19 +++++
 b/apps/misc.h                        |    2 
 b/lib/rbcodec/dsp/dsp.c              |  118 ++++++++++++++++-------------------
 b/lib/rbcodec/dsp/dsp.h              |    9 ++
 6 files changed, 106 insertions(+), 71 deletions(-)

   0004-rbcodec-refactoring-timestretch-buffer-alloc... (7 KiB) 
 b/apps/main.c               |    8 --------
 b/apps/settings.c           |    8 +++++++-
 b/apps/settings_list.c      |    2 +-
 b/lib/rbcodec/dsp/dsp.c     |   16 ++++++++++++----
 b/lib/rbcodec/dsp/dsp.h     |    3 ++-
 b/lib/rbcodec/dsp/tdspeed.c |   28 +++++++++++++++-------------
 b/lib/rbcodec/dsp/tdspeed.h |    3 ++-
 b/lib/rbcodec/test/warble.c |    3 +--
 8 files changed, 40 insertions(+), 31 deletions(-)

   0005-rbcodec-refactoring-get_audio_base_data_type... (4 KiB) 
 b/apps/playback.c                 |    9 ++++-----
 b/lib/rbcodec/metadata/metadata.c |   12 ++++--------
 b/lib/rbcodec/metadata/metadata.h |    2 +-
 b/lib/rbcodec/test/warble.c       |    2 +-
 4 files changed, 10 insertions(+), 15 deletions(-)

   0006-rbcodec-refactoring-autoresumable.patch (5.2 KiB) 
 b/apps/playback.c                 |   58 +++++++++++++++++++++++++++++++++
 b/lib/rbcodec/metadata/metadata.c |   65 --------------------------------------
 b/lib/rbcodec/metadata/metadata.h |    4 --
 3 files changed, 58 insertions(+), 69 deletions(-)

This task depends upon

Comment by Nils Wallménius (nls) - Saturday, 27 August 2011, 14:16 GMT+2
in patch 0004 it looks like the code you added in settings_apply() can be run when for example loading a config which would cause problems (both from buffer alloc and the tdspeed_init() function that is marked with INIT_ATTR) it needs more guarding to only run at boot.
Comment by Michael Sevakis (MikeS) - Sunday, 28 August 2011, 09:59 GMT+2
Keep base data type and the DSP should not over-yield. It used to yield each iteration with was overkill. If these changes are for lib for preemtive threading, just kill the yields. I don't want regression nor functions to become overly specialized.

Edit: Nils was kind enough to remind me it's being made a lib that even the main rockbox build relies upon. I guess the reliance on the tick shouldn't be an issue in that case, and yields unneeded as a standalone.
Comment by Michael Sevakis (MikeS) - Sunday, 28 August 2011, 10:09 GMT+2
Booleans are horrendous things for the most part. Really, the lib should include data types, especially if the playback engine were to get lib-ized, which would be a good idea so that it can be front-ended with native UI. It needs to remain forward-looking instead of merely expedient in its implementation.

Loading...