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



Rockbox mail archive

Subject: Some questions about naming of some functions

Some questions about naming of some functions

From: Christian Gmeiner <christian.gmeiner_at_gmail.com>
Date: Sat, 9 Feb 2008 16:20:09 +0100

HI all,

as I have more time for rockbox now (term at university is over and I have
some holidays), I want to finish my started work of "audio hardware
unification".
The goal of my work should be to clean up firmware/sound.c and minimize the
usage of #if defined(...). At the moment I am looking at the volume
functions
with their different names and different usages.

Lets start :)

- firmware/sound.c -

static void set_prescaled_volume(void)
{
    int prescale = 0;
    int l, r;

/* The WM codecs listed don't have suitable prescaler functionality, so we
let
 * the prescaler stay at 0 for these unless SW tone controls are in use */
#if defined(HAVE_SW_TONE_CONTROLS) || !(defined(HAVE_WM8975) \
    || defined(HAVE_WM8731) || defined(HAVE_WM8721) || defined(HAVE_WM8751)
\
    || defined(HAVE_WM8758))

    prescale = MAX(current_bass, current_treble);
    if (prescale < 0)
        prescale = 0; /* no need to prescale if we don't boost
                          bass or treble */

    /* Gain up the analog volume to compensate the prescale gain reduction,
     * but if this would push the volume over the top, reduce prescaling
     * instead (might cause clipping). */
    if (current_volume + prescale > VOLUME_MAX)
        prescale = VOLUME_MAX - current_volume;
#endif

#if defined(HAVE_SW_TONE_CONTROLS)
    dsp_callback(DSP_CALLBACK_SET_PRESCALE, prescale);
#elif CONFIG_CODEC == MAS3507D
    mas_writereg(MAS_REG_KPRESCALE, prescale_table[prescale/10]);
#elif defined(HAVE_UDA1380)
    audiohw_set_mixer_vol(tenthdb2mixer(-prescale),
tenthdb2mixer(-prescale));
#endif

    if (current_volume == VOLUME_MIN)
        prescale = 0; /* Make sure the chip gets muted at VOLUME_MIN */

    l = r = current_volume + prescale;

    if (current_balance > 0)
    {
        l -= current_balance;
        if (l < VOLUME_MIN)
            l = VOLUME_MIN;
    }
    if (current_balance < 0)
    {
        r += current_balance;
        if (r < VOLUME_MIN)
            r = VOLUME_MIN;
    }

#if CONFIG_CODEC == MAS3507D
    dac_volume(tenthdb2reg(l), tenthdb2reg(r), false);
#elif defined(HAVE_UDA1380) || defined(HAVE_WM8975) || defined(HAVE_WM8758)
\
   || defined(HAVE_WM8731) || defined(HAVE_WM8721) || defined(HAVE_WM8751) \
   || defined(HAVE_AS3514)
    audiohw_set_master_vol(tenthdb2master(l), tenthdb2master(r));
#if defined(HAVE_WM8975) || defined(HAVE_WM8758) \
   || (defined(HAVE_WM8751) && !defined(MROBE_100))
    audiohw_set_lineout_vol(tenthdb2master(0), tenthdb2master(0));
#endif

#elif defined(HAVE_TLV320)
    audiohw_set_headphone_vol(tenthdb2master(l), tenthdb2master(r));
#endif
}
#endif /* (CONFIG_CODEC == MAS3507D) || defined HAVE_UDA1380 */

Lets discuss used functions and why they are used in this context. Before I
start... I must admit that I do not know what prescaling means in the
context
of audio - maybe somebody can help me here out.

The first part of this function sets the prescale value.

uda1380:
    audiohw_set_mixer_vol(tenthdb2mixer(-prescale),
tenthdb2mixer(-prescale));
    As I do not know what prescaling means it seems that we need to change
the mixer value
    of the uda1380 chip... why don't we need this on the other chips?
    Can we rename this part to audiohw_set_prescaler?

The second part changes the real volume the the output which we can hear
through our headphones.

Here we have three variations:
* audiohw_set_master_vol
* audiohw_set_lineout_vol
* audiohw_set_headphone_vol

which do the same... setting volume for our headphones. The question for me
is now, if we need all these different functions, which
have the same result in the end? Is it important for the caller of such a
function to know what technique we use to change volume (set headphone
registers,
set master volume registers, set lineout registers)?
I think no! As a result of this we could have one unique function name, e.g.
audiohw_set_volume(l, r) to change the volume.

Please tell me about your opinions and maybe you can give me an
understandable url/document, which explains what prescale does.

Thanks,
Christian
Received on 2008-02-09


Page was last modified "Jan 10 2012" The Rockbox Crew
aaa