Re: [Qemu-devel] [PATCH 18/25] audio: do not run each backend in audio_run

[Kővágó Zoltán]

Hi

2015-08-20 22:49 keltezéssel, Marc-André Lureau írta:
> Hi
>
> On Thu, Aug 6, 2015 at 8:28 PM, Kővágó, Zoltán <address@hidden> wrote:
> > audio_run is called manually by alsa and oss backends when polling.
> > In this case only the requesting backend should be run, not all of them.
> >
> > Signed-off-by: Kővágó, Zoltán <address@hidden>
> > ---
> >   audio/alsaaudio.c |  7 +++++--
> >   audio/audio.c     | 17 +++++++----------
> >   audio/audio_int.h |  2 +-
> >   audio/ossaudio.c  | 12 ++++++------
> >   4 files changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
> > index cfe4aec..0750d0d 100644
> > --- a/audio/alsaaudio.c
> > +++ b/audio/alsaaudio.c
> > @@ -40,6 +40,7 @@ struct pollhlp {
> >       struct pollfd *pfds;
> >       int count;
> >       int mask;
> > +    AudioState *s;
>
> Instead of adding that pointer, you could lookup the hw->s using container_of()

I'm afraid that won't work.  The pollhlp is used both inside ALSAVoiceIn 
and ALSAVoiceOut, but they have a common callback.

> >   };
> >
> >   typedef struct ALSAVoiceOut {
> > @@ -200,11 +201,11 @@ static void alsa_poll_handler (void *opaque)
> >           break;
> >
> >       case SND_PCM_STATE_PREPARED:
> > -        audio_run ("alsa run (prepared)");
> > +        audio_run(hlp->s, "alsa run (prepared)");
> >           break;
> >
> >       case SND_PCM_STATE_RUNNING:
> > -        audio_run ("alsa run (running)");
> > +        audio_run (hlp->s, "alsa run (running)");
> >           break;
> >
> >       default:
> > @@ -758,6 +759,7 @@ static int alsa_init_out(HWVoiceOut *hw, struct audsettings 
> > *as,
> >           return -1;
> >       }
> >
> > +    alsa->pollhlp.s = hw->s;
> >       alsa->handle = handle;
> >       alsa->dev = dev;
> >       return 0;
> > @@ -859,6 +861,7 @@ static int alsa_init_in(HWVoiceIn *hw, struct audsettings 
> > *as, void *drv_opaque)
> >           return -1;
> >       }
> >
> > +    alsa->pollhlp.s = hw->s;
> >       alsa->handle = handle;
> >       alsa->dev = dev;
> >       return 0;
> > diff --git a/audio/audio.c b/audio/audio.c
> > index 7468b94..e213deb 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -790,8 +790,9 @@ static void audio_reset_timer (AudioState *s)
> >
> >   static void audio_timer (void *opaque)
> >   {
> > -    audio_run ("timer");
> > -    audio_reset_timer (opaque);
> > +    AudioState *s = opaque;
> > +    audio_run(s, "timer");
> > +    audio_reset_timer(s);
> >   }
> >
> >   /*
> > @@ -1194,15 +1195,11 @@ static void audio_run_capture (AudioState *s)
> >       }
> >   }
> >
> > -void audio_run (const char *msg)
> > +void audio_run(AudioState *s, const char *msg)
> >   {
> > -    AudioState *s;
> > -
> > -    QTAILQ_FOREACH(s, &audio_states, list) {
> > -        audio_run_out (s);
> > -        audio_run_in (s);
> > -        audio_run_capture (s);
> > -    }
> > +    audio_run_out(s);
> > +    audio_run_in(s);
> > +    audio_run_capture(s);
> >
> >   #ifdef DEBUG_POLL
> >       {
> > diff --git a/audio/audio_int.h b/audio/audio_int.h
> > index 1d81658..c52c656 100644
> > --- a/audio/audio_int.h
> > +++ b/audio/audio_int.h
> > @@ -220,7 +220,7 @@ int audio_pcm_hw_clip_out (HWVoiceOut *hw, void *pcm_buf,
> >   int audio_bug (const char *funcname, int cond);
> >   void *audio_calloc (const char *funcname, int nmemb, size_t size);
> >
> > -void audio_run (const char *msg);
> > +void audio_run(AudioState *s, const char *msg);
> >
> >   #define VOICE_ENABLE 1
> >   #define VOICE_DISABLE 2
> > diff --git a/audio/ossaudio.c b/audio/ossaudio.c
> > index a5e7f7c..9eae769 100644
> > --- a/audio/ossaudio.c
> > +++ b/audio/ossaudio.c
> > @@ -112,28 +112,28 @@ static void oss_anal_close (int *fdp)
> >
> >   static void oss_helper_poll_out (void *opaque)
> >   {
> > -    (void) opaque;
> > -    audio_run ("oss_poll_out");
> > +    AudioState *s = opaque;
> > +    audio_run(s, "oss_poll_out");
> >   }
> >
> >   static void oss_helper_poll_in (void *opaque)
> >   {
> > -    (void) opaque;
> > -    audio_run ("oss_poll_in");
> > +    AudioState *s = opaque;
> > +    audio_run(s, "oss_poll_in");
> >   }
> >
> >   static void oss_poll_out (HWVoiceOut *hw)
> >   {
> >       OSSVoiceOut *oss = (OSSVoiceOut *) hw;
> >
> > -    qemu_set_fd_handler (oss->fd, NULL, oss_helper_poll_out, NULL);
> > +    qemu_set_fd_handler(oss->fd, NULL, oss_helper_poll_out, hw->s);
> >   }
> >
> >   static void oss_poll_in (HWVoiceIn *hw)
> >   {
> >       OSSVoiceIn *oss = (OSSVoiceIn *) hw;
> >
> > -    qemu_set_fd_handler (oss->fd, oss_helper_poll_in, NULL, NULL);
> > +    qemu_set_fd_handler(oss->fd, oss_helper_poll_in, NULL, hw->s);
> >   }
> >
> >   static int oss_write (SWVoiceOut *sw, void *buf, int len)
> > --
> > 2.4.5
>
> other than that,
> Reviewed-by: Marc-André Lureau <address@hidden>