FS#12240 - rbcodec refactoring part 1

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


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.
This task depends upon

Comment by Nils Wallménius (nls) - Saturday, 27 August 2011, 12:16 GMT
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, 07:59 GMT
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, 08:09 GMT
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.