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

[Kővágó Zoltán]

2015-06-17 10:01 keltezéssel, Markus Armbruster írta:
> "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?

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";
> >   }
> [...]