release
dev builds
extras
themes manual
wiki
device status forums
mailing lists
IRC bugs
patches
dev guide



Search | Go
Wiki > Main > AudioHardwareAPIProposal (r17)

Audio Hardware API

Why

Maybe you think what the goal of an audio hardware api is. Its very easy, we

  • ...will get a common interface to access the audio hardware
  • ...will get rid of some #ifdef's
  • ...will get a cleaned up sound.c
  • ...will have all audio codec realated informations at one point

The API

warning At the moment Its only for software based players. But adding support for full hardware codecs isnt hard, because we need to add only stuff like superbass and rewrite the old code as a audio codec warning

enum {
    SOUND_VOLUME = 0,
    SOUND_BASS,
    SOUND_TREBLE,
    SOUND_BALANCE,
    SOUND_CHANNELS,
    SOUND_STEREO_WIDTH,
};

enum Channel {
    SOUND_CHAN_STEREO = 0,
    SOUND_CHAN_MONO,
    SOUND_CHAN_CUSTOM,
    SOUND_CHAN_MONO_LEFT,
    SOUND_CHAN_MONO_RIGHT,
    SOUND_CHAN_KARAOKE,
    SOUND_CHAN_NUM_MODES,
};

enum OutDestination {
    OUTPUT_HEADPHONES = 0,
    OUTPUT_LINEOUT,
    OUTPUT_SPDIF,
};

enum RecSource {
    REC_SOURCE_MIC = 0,
    REC_SOURCE_LINEIN,
    REC_SOURCE_FM,
    REC_SOURCE_SPDIF,
};

function a must have description
Lifecyle
void audiohw_init(void) y Initialize all needed registers
void audiohw_postinit(void) y Do some stuff directly after init
void audiohw_shutdown(void) y Shutdown audio codec
void audiohw_reset(void) y Reset audio codec into a well defined state
Playback
void audiohw_enable_output(bool enable) y Poweron/Unmute parts of the audio codec
void audiohw_set_output_destination(enum OutDestination? destination) y Define where to output audio
void audiohw_mute(bool mute) y Mute the current active output destination
Sound settings
void audiohw_set_volume(int l, int r) y Set the volume of the current output destination - l and r are tenth of dB
void audiohw_set_bass(int value) n Set bass in audio codec
void audiohw_set_treble(int value) n Set treble in audio codec
void audiohw_set_channel(enum Channel value) n Set channel configuration in audio codec
void audiohw_set_stereo_width(int value) n Set stereo width in audio codec
audiohw_set_prescale n Set prescale in audio codec
void audiohw_set_frequency(int freq) ? Set frequency in audio codec
Recording (#ifdef HAVE_RECORDING of course)
void audiohw_enable_recording(bool enable) y Enable/Disable recording
void audiohw_set_rec_source(enum RecSource? source) y Define from which source to record
void audiohw_enable_rec_monitor(bool enable) ? Should the current recorded data played
void audiohw_set_rec_volume(int r, int l, int type) y r and l in tenth of dB

The changes

Define all possible audio hardware functions in one place - audiohw.h

I think that it will be also a good idea, to put
#define HW_SAMPR_CAPS   (SAMPR_CAP_88 | SAMPR_CAP_44 | SAMPR_CAP_22 | SAMPR_CAP_11)
#define REC_SAMPR_CAPS  (SAMPR_CAP_44 | SAMPR_CAP_22 | SAMPR_CAP_11)
directly into the driver.

Also we could add some #defines to each platform, to define the supported output and recording destinations/sources.

How could it look in the driver

The good thing about the new architecture is that we dont need to change much code in existing drivers. We only need to change the function names to match the one defined in audiohw.h

Add metainformations into audio codec c file, like

const struct sound_settings_info audiohw_settings[] = {
    [SOUND_VOLUME] = {"dB", 0,  1, -40,   6, -25},
    [SOUND_BASS] = {"dB", 0,  1, -24,  24,   0},
    [SOUND_TREBLE] = {"dB", 0,  1, -24,  24,   0},
    [SOUND_BALANCE] = {"%",  0,  1,-100, 100,   0},
    [SOUND_CHANNELS] = {"",   0,  1,   0,   5,   0},
    [SOUND_STEREO_WIDTH]  = {"%",  0,  1,   0, 255, 100},
}; 

Changes in sound.c

Comment from MarcoenHirschberg - 30 Apr 2007: I was more thinking of having defines for the capabilities in each codec's header files. This would save binary size and make the code faster:

void sound_set_treble(int value) {

    if (!audio_is_initialized) {
        return;
    }

    current_treble = value * 10;

#ifdef AUDIOHW_HAVE_TREBBLE
    audiohw_set_treble(current_treble);
#else
    dsp_callback(DSP_CALLBACK_SET_TREBLE, current_treble);
#endif

    set_prescaled_volume();
}

What is the reason you want to have it in a struct? To me that would only make sense on a DAP with multiple DACs with different capabilities.

Comment from ChristianGmeiner - 30 Apr 2007: The reason I want it in a struct, is to get some kind of object-orientation into RockBox's core. At the moment some codecs use different function names for the same kind of functionality (e.g. set volume) and as a result of it we use a lot of ifdefines, with the struct we go for sure that we have one unique interface to the hardware. An other argument for the struct is that we have everything connected to the audio codec is at one place, to name it: in the struct. We can access soundsetting ranges and other needed informations. Also I think it is a good idea to get rid of too many ifdefines. When we would switch to something like AUDIOHW_HAVE_TREBBLE, AUDIOHW_HAVE_BASS, we might save some bytes, but on the other hand we introduce a handful defines.

Things to do

  • warning tell me your comments warning
  • review api
  • write a proof-of-concept (mostly done)
  • introduce the new api for sw based players via ifdef
  • write audio drivers for hw based players
  • remove all not needed ifdefs

Discussion

What about this cant be done using the target tree? Don't we want the audio control code running as fast as possible? adding this "OO" layer would slow it down wouldnt it? -- JonathanGordon - 01 May 2007

Comment from ChristianGmeiner - 1 May 2007: I dont think that a little if will make a big difference in speed. Sure we want it to be as fast as possible but in my mind we sould optimize the upper layers, which use the audio hardware, such as codecs, the way we handle the audio from buffering over decoding. Also other embedded operationg systems use some structs with function pointers or even more "OO". To name it: Linux, Symbian OS (if you need more examples, tell it me and I will list some more). The goal for me with this proposal is to define a standard of function names, how to put the codecs metainformations (supports XXX, YYY) directly into the codec, how to clean up all the ifdefs and make it easy to maintain and extend. So give me some good points against the "OO" thing - telling me "I dont like OO" does not count. And I dont say MarcoenHirschberg's idea is bad or that I dont like it. I can also live with the #define idea.

-- I understand what your trying to do and why, I just disagree with the way your doing it... If the only reason your using the struct is to handle all the different function names, why not just use standard names (similar to how tartget tree does it...) just stick the valid set of functions in a .h file and force the different sound drivers to conform. As long as only one driver is compiled for each target this would work fine, and without the overhead of the function pointer struct. -- JonathanGordon - 02 May 2007

I'm all for the fixed ifdeffed functionnames, to reduce codesize as much as possible. Your system falls into the category variables won't, constants aren't - you're creating a variable system for stuff that's pretty much fixed per target. -- PeterDHoye - 01 May 2007

Okay guys.. I have changes my code to use ifdef's and I must say that it looks very cool smile I will post here later the audiohw_api and a patch for one target. ChristianGmeiner - 02 May 2007

ChristianGmeiner - 02 May 2007: Updateded everything. Please review the api (audiohw_*) and tell me what to make better.

Looks much better smile The last thing I'd change is name each of the enums and use that in the funciton parameter list instead of int. Also is "dB" needed in the setting struct? do any targets not use "dB"? -- JonathanGordon - 03 May 2007

The "dB" stuff is needed.. look at upper... i have added something smile ChristianGmeiner - 03 May 2007

Thanks Christian, this is what I had in mind when I changed all the codec functions to audiohw_* in the first place. MarcoenHirschberg - 03 May 2007

Patches

This is the first patch, which is only for sansa and Simulator. Also note that the changes are in the firmware and that upper layers dont use them fully. Recording is not ready, as I need to look deeper in this area. Have a look at it and tell me what you think - http://www.rockbox.org/tracker/task/7138 - ChristianGmeiner - 09 May 2007
Edit | Attach | Print version | History: r20 | r18 < r17 < r16 < r15 | Backlinks | View wiki text | More topic actions...
r17 - 09 May 2007 - 18:02:08 - ChristianGmeiner
Copyright by the contributing authors.