[Alsa-user] are ALSA sources written accor ding to "single point of change" concep t ?

[Sergei Steshenko]

Hello Developers and All,

quick grep on ALSA sources shows, for example, this:

alsa-lib-1.0.17a/src/pcm/pcm_dsnoop.c :

    765 int _snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
    766                        snd_config_t *root, snd_config_t *conf,
    767                        snd_pcm_stream_t stream, int mode)
    768 {
    769         snd_config_t *sconf;
    770         struct slave_params params;
    771         struct snd_pcm_direct_open_conf dopen;
    772         int bsize, psize;
    773         int err;
    774
    775         err = snd_pcm_direct_parse_open_conf(root, conf, stream, 
&dopen);
    776         if (err < 0)
    777                 return err;
    778
    779         /* the default settings, it might be invalid for some hardware 
*/
    780         params.format = SND_PCM_FORMAT_S16;
    781         params.rate = 48000;
    782         params.channels = 2;
    783         params.period_time = -1;
    784         params.buffer_time = -1;
    785         bsize = psize = -1;
    786         params.periods = 3;
    787         err = snd_pcm_slave_conf(root, dopen.slave, &sconf, 8,
    788                                  SND_PCM_HW_PARAM_FORMAT, 
SCONF_UNCHANGED, &params.format,
    789                                  SND_PCM_HW_PARAM_RATE, 0, &params.rate,
    790                                  SND_PCM_HW_PARAM_CHANNELS, 0, 
&params.channels,
    791                                  SND_PCM_HW_PARAM_PERIOD_TIME, 0, 
&params.period_time,
    792                                  SND_PCM_HW_PARAM_BUFFER_TIME, 0, 
&params.buffer_time,
    793                                  SND_PCM_HW_PARAM_PERIOD_SIZE, 0, 
&psize,
    794                                  SND_PCM_HW_PARAM_BUFFER_SIZE, 0, 
&bsize,
    795                                  SND_PCM_HW_PARAM_PERIODS, 0, 
&params.periods);
    796         if (err < 0)
    797                 return err;
    798
    799         /* set a reasonable default */
    800         if (psize == -1 && params.period_time == -1)
    801                 params.period_time = 125000;    /* 0.125 seconds */
    802
    803         if (params.format == -2)
    804                 params.format = SND_PCM_FORMAT_UNKNOWN;
    805
    806         params.period_size = psize;
    807         params.buffer_size = bsize;
    808
    809         err = snd_pcm_dsnoop_open(pcmp, name, &dopen, &params,
    810                                   root, sconf, stream, mode);
    811         snd_config_delete(sconf);
    812         return err;
    813 }
.


I question this line:

    781         params.rate = 48000;
.

In my opinions this is wrong.

It's wrong in a sense the whole ALSA should have just _one_ place where default 
sample rate
is defined, and just _one_ mechanism through which default sample rate is set 
(like through
.asoundrc / alsa.conf / /usr/share/alsa/alsa.conf .

As I was taught, numeric constants other than 0, 1, maybe 2 are not allowed in 
source code, instead the
constants should all have meaningful names (e.g. "C" macros, C++ constant class 
data members, etc.) and
these names and not numeric constants should be used in the code.

Thanks,
  Sergei.


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Alsa-user mailing list
Alsa-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-user