Re: [Qemu-devel] [PATCH v2 5/6] audio: use qapi AudioFormat instead of audfmt_e

["Kővágó Zoltán" <dirty.ice.hu@gmail.com>]

2015-06-17 10:01 keltezéssel, Markus Armbruster írta:
> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> writes:
>
>> I had to include an enum for audio sampling formats into qapi, but that meant
>> duplicating the audfmt_e enum. This patch replaces audfmt_e and associated
>> values with the qapi generated AudioFormat enum.
>>
>> This patch is mostly a search-and-replace, except for switches where the qapi
>> generated AUDIO_FORMAT_MAX caused problems.
> [...]
>> diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
>> index 6315b2d..4d38f5d 100644
>> --- a/audio/alsaaudio.c
>> +++ b/audio/alsaaudio.c
> [...]
>> @@ -639,19 +639,22 @@ static int alsa_open (int in, struct alsa_params_req *req,
>>           bytes_per_sec = freq << (nchannels == 2);
>>
>>           switch (obt->fmt) {
>> -        case AUD_FMT_S8:
>> -        case AUD_FMT_U8:
>> +        case AUDIO_FORMAT_S8:
>> +        case AUDIO_FORMAT_U8:
>>               break;
>>
>> -        case AUD_FMT_S16:
>> -        case AUD_FMT_U16:
>> +        case AUDIO_FORMAT_S16:
>> +        case AUDIO_FORMAT_U16:
>>               bytes_per_sec <<= 1;
>>               break;
>>
>> -        case AUD_FMT_S32:
>> -        case AUD_FMT_U32:
>> +        case AUDIO_FORMAT_S32:
>> +        case AUDIO_FORMAT_U32:
>>               bytes_per_sec <<= 2;
>>               break;
>> +
>> +        case AUDIO_FORMAT_MAX:
>> +            break;
>
> Can this happen?

Not under normal circumstances, but gcc warns otherwise.

>>           }
>>
>>           threshold = (conf->threshold * bytes_per_sec) / 1000;
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 5be4b15..112b57b 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -75,7 +75,7 @@ static struct {
>>           .settings = {
>>               .freq = 44100,
>>               .nchannels = 2,
>> -            .fmt = AUD_FMT_S16,
>> +            .fmt = AUDIO_FORMAT_S16,
>>               .endianness =  AUDIO_HOST_ENDIANNESS,
>>           }
>>       },
>> @@ -87,7 +87,7 @@ static struct {
>>           .settings = {
>>               .freq = 44100,
>>               .nchannels = 2,
>> -            .fmt = AUD_FMT_S16,
>> +            .fmt = AUDIO_FORMAT_S16,
>>               .endianness = AUDIO_HOST_ENDIANNESS,
>>           }
>>       },
>> @@ -219,58 +219,61 @@ static char *audio_alloc_prefix (const char *s)
>>       return r;
>>   }
>>
>> -static const char *audio_audfmt_to_string (audfmt_e fmt)
>> +static const char *audio_audfmt_to_string (AudioFormat fmt)
>>   {
>>       switch (fmt) {
>> -    case AUD_FMT_U8:
>> +    case AUDIO_FORMAT_U8:
>>           return "U8";
>>
>> -    case AUD_FMT_U16:
>> +    case AUDIO_FORMAT_U16:
>>           return "U16";
>>
>> -    case AUD_FMT_S8:
>> +    case AUDIO_FORMAT_S8:
>>           return "S8";
>>
>> -    case AUD_FMT_S16:
>> +    case AUDIO_FORMAT_S16:
>>           return "S16";
>>
>> -    case AUD_FMT_U32:
>> +    case AUDIO_FORMAT_U32:
>>           return "U32";
>>
>> -    case AUD_FMT_S32:
>> +    case AUDIO_FORMAT_S32:
>>           return "S32";
>> +
>> +    case AUDIO_FORMAT_MAX:
>
> default: would be more defensive.  Same elsewhere.

Ok, I'll change them to default.

>> +        abort();
>>       }
>>
>>       dolog ("Bogus audfmt %d returning S16\n", fmt);
>>       return "S16";
>>   }
> [...]
>