On Tue, 01 May 2018 11:14:56 +0200,
Takashi Sakamoto wrote:
>
> Hi,
>
> On May 1 2018 15:54, Takashi Iwai wrote:
> > On Tue, 01 May 2018 05:02:09 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> Hi,
> >>
> >> On May 1 2018 00:10, Takashi Iwai wrote:
> >>> On Sun, 29 Apr 2018 08:50:23 +0200,
> >>> Takashi Sakamoto wrote:
> >>>>
> >>>> In former commits, proxy structure get members for cache of stream
> >>>> formats. This commit fills the cache with stream formats at current mode
> >>>> of sampling transmission frequency.
> >>>>
> >>>> Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
> >>>> ---
> >>>> sound/firewire/dice/dice-stream.c | 76 +++++++++++++++++++++++++++++++++++++++
> >>>> sound/firewire/dice/dice.c | 4 +++
> >>>> sound/firewire/dice/dice.h | 3 ++
> >>>> 3 files changed, 83 insertions(+)
> >>>>
> >>>> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
> >>>> index 928a255bfc35..2a9f0cd994a5 100644
> >>>> --- a/sound/firewire/dice/dice-stream.c
> >>>> +++ b/sound/firewire/dice/dice-stream.c
> >>>> @@ -30,6 +30,24 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
> >>>> [6] = 192000,
> >>>> };
> >>>> +int snd_dice_stream_get_rate_mode(struct snd_dice *dice,
> >>>> unsigned int rate,
> >>>> + unsigned int *mode)
> >>>> +{
> >>>> + int i;
> >>>> +
> >>>> + for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
> >>>> + if (!(dice->clock_caps & BIT(i)))
> >>>> + continue;
> >>>> + if (snd_dice_rates[i] != rate)
> >>>> + continue;
> >>>> +
> >>>> + *mode = (i - 1) / 2;
> >>>
> >>> What if i=0? It'll be a negative value?
> >>
> >> Yes. But division by 2 to -1 results in 0 because in C language
> >> specification 'truncate toward zero'[1] is applied to discard
> >> fractional part. Then, the result is assigned to value of 'unsigned int'
> >> type. As a result:
> >>
> >> 0: 32000/44100/48000
> >> 1: 88200/96000
> >> 2: 176400/192000
> >>
> >> This is what I expect.
> >>
> >> [1] footnote for '6.5.5 Multiplicative operators' clause in ISO/IEC 9899.
> >
> > This is too tricky. The result would change dramatically when i were
> > declared as unsigned.
> >
> > And I can think of someone trying to change it unsigned because of the
> > comparison against ARRAY_SIZE() (we've got gcc warnings for that in
> > the past).
> >
> > Please make either it more robust or put a proper comment.
>
> Hm. Any comment includes ambiguity, so I prefer the robust
> representation with more consumption of .rodata section.
>
> ```
> int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
> enum snd_dice_rate_mode *mode)
> {
> /* Corresponding to each entry in snd_dice_rates. */
> static const enum snd_dice_rate_mode modes[] = {
> [0] = SND_DICE_RATE_MODE_LOW,
> [1] = SND_DICE_RATE_MODE_LOW,
> [2] = SND_DICE_RATE_MODE_LOW,
> [3] = SND_DICE_RATE_MODE_MIDDLE,
> [4] = SND_DICE_RATE_MODE_MIDDLE,
> [5] = SND_DICE_RATE_MODE_HIGH,
> [6] = SND_DICE_RATE_MODE_HIGH,
> };
> int i;
>
> for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
> if (!(dice->clock_caps & BIT(i)))
> continue;
> if (snd_dice_rates[i] != rate)
> continue;
>
> *mode = modes[i];
> return 0;
> }
>
> return -EINVAL;
> }
> ```
That's one way, yes.
(BTW, you can write down in a style
[0 ... 2] = SND_DICE_RATE_MODE_LOW,
too.)
Or, deal the exception explicity, e.g.
*mode = i > 0 ? ((i - 1) / 2) : 0;
Or, mention just that i=0 shall give mode=0 with signed int.
I don't mind either way.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
