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

[Markus Armbruster]

"Kővágó, Zoltán" <address@hidden> 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?

>          }
>  
>          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.

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