Re: [Qemu-devel] [PATCH 11/25] audio: reduce glob_audio_state usage

[Marc-André Lureau]

Hi

On Thu, Aug 6, 2015 at 8:28 PM, Kővágó, Zoltán <address@hidden> wrote:
> Signed-off-by: Kővágó, Zoltán <address@hidden>

Fill a bit the commit message, to say simply use AudioState instead.
Also explain why you add AudioState *s in struct although it's not
used in that commit, saying simply it's going to be used in later
commit is enough imho (I guess it is).

> ---
>  audio/audio.c          | 41 ++++++++++++++++++++---------------------
>  audio/audio_int.h      |  4 ++++
>  audio/audio_template.h | 46 ++++++++++++++++++++++------------------------
>  3 files changed, 46 insertions(+), 45 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 08ac15e..05b24dc 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -367,12 +367,10 @@ static void noop_conv (struct st_sample *dst, const 
> void *src, int samples)
>      (void) samples;
>  }
>
> -static CaptureVoiceOut *audio_pcm_capture_find_specific (
> -    struct audsettings *as
> -    )
> +static CaptureVoiceOut *audio_pcm_capture_find_specific(AudioState *s,
> +                                                        struct audsettings 
> *as)
>  {
>      CaptureVoiceOut *cap;
> -    AudioState *s = &glob_audio_state;
>
>      for (cap = s->cap_head.lh_first; cap; cap = cap->entries.le_next) {
>          if (audio_pcm_info_eq (&cap->hw.info, as)) {
> @@ -449,7 +447,7 @@ static void audio_detach_capture (HWVoiceOut *hw)
>
>  static int audio_attach_capture (HWVoiceOut *hw)
>  {
> -    AudioState *s = &glob_audio_state;
> +    AudioState *s = hw->s;
>      CaptureVoiceOut *cap;
>
>      audio_detach_capture (hw);
> @@ -762,15 +760,15 @@ static void audio_pcm_print_info (const char *cap, 
> struct audio_pcm_info *info)
>  /*
>   * Timer
>   */
> -static int audio_is_timer_needed (void)
> +static int audio_is_timer_needed(AudioState *s)
>  {
>      HWVoiceIn *hwi = NULL;
>      HWVoiceOut *hwo = NULL;
>
> -    while ((hwo = audio_pcm_hw_find_any_enabled_out (hwo))) {
> +    while ((hwo = audio_pcm_hw_find_any_enabled_out(s, hwo))) {
>          if (!hwo->poll_mode) return 1;
>      }
> -    while ((hwi = audio_pcm_hw_find_any_enabled_in (hwi))) {
> +    while ((hwi = audio_pcm_hw_find_any_enabled_in(s, hwi))) {
>          if (!hwi->poll_mode) return 1;
>      }
>      return 0;
> @@ -778,7 +776,7 @@ static int audio_is_timer_needed (void)
>
>  static void audio_reset_timer (AudioState *s)
>  {
> -    if (audio_is_timer_needed ()) {
> +    if (audio_is_timer_needed(s)) {
>          timer_mod (s->ts,
>              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->period_ticks);
>      }
> @@ -847,7 +845,7 @@ void AUD_set_active_out (SWVoiceOut *sw, int on)
>
>      hw = sw->hw;
>      if (sw->active != on) {
> -        AudioState *s = &glob_audio_state;
> +        AudioState *s = sw->s;
>          SWVoiceOut *temp_sw;
>          SWVoiceCap *sc;
>
> @@ -894,7 +892,7 @@ void AUD_set_active_in (SWVoiceIn *sw, int on)
>
>      hw = sw->hw;
>      if (sw->active != on) {
> -        AudioState *s = &glob_audio_state;
> +        AudioState *s = sw->s;
>          SWVoiceIn *temp_sw;
>
>          if (on) {
> @@ -1017,7 +1015,7 @@ static void audio_run_out (AudioState *s)
>      HWVoiceOut *hw = NULL;
>      SWVoiceOut *sw;
>
> -    while ((hw = audio_pcm_hw_find_any_enabled_out (hw))) {
> +    while ((hw = audio_pcm_hw_find_any_enabled_out(s, hw))) {
>          int played;
>          int live, free, nb_live, cleanup_required, prev_rpos;
>
> @@ -1121,7 +1119,7 @@ static void audio_run_in (AudioState *s)
>  {
>      HWVoiceIn *hw = NULL;
>
> -    while ((hw = audio_pcm_hw_find_any_enabled_in (hw))) {
> +    while ((hw = audio_pcm_hw_find_any_enabled_in(s, hw))) {
>          SWVoiceIn *sw;
>          int captured, min;
>
> @@ -1224,8 +1222,8 @@ static int audio_driver_init(AudioState *s, struct 
> audio_driver *drv,
>      s->drv_opaque = drv->init(dev);
>
>      if (s->drv_opaque) {
> -        audio_init_nb_voices_out (drv);
> -        audio_init_nb_voices_in (drv);
> +        audio_init_nb_voices_out(s, drv);
> +        audio_init_nb_voices_in(s, drv);
>          s->drv = drv;
>          return 0;
>      }
> @@ -1244,11 +1242,11 @@ static void audio_vm_change_state_handler (void 
> *opaque, int running,
>      int op = running ? VOICE_ENABLE : VOICE_DISABLE;
>
>      s->vm_running = running;
> -    while ((hwo = audio_pcm_hw_find_any_enabled_out (hwo))) {
> +    while ((hwo = audio_pcm_hw_find_any_enabled_out(s, hwo))) {
>          hwo->pcm_ops->ctl_out (hwo, op);
>      }
>
> -    while ((hwi = audio_pcm_hw_find_any_enabled_in (hwi))) {
> +    while ((hwi = audio_pcm_hw_find_any_enabled_in(s, hwi))) {
>          hwi->pcm_ops->ctl_in (hwi, op);
>      }
>      audio_reset_timer (s);
> @@ -1260,7 +1258,7 @@ static void audio_atexit (void)
>      HWVoiceOut *hwo = NULL;
>      HWVoiceIn *hwi = NULL;
>
> -    while ((hwo = audio_pcm_hw_find_any_out (hwo))) {
> +    while ((hwo = audio_pcm_hw_find_any_out(s, hwo))) {
>          SWVoiceCap *sc;
>
>          if (hwo->enabled) {
> @@ -1278,7 +1276,7 @@ static void audio_atexit (void)
>          }
>      }
>
> -    while ((hwi = audio_pcm_hw_find_any_in (hwi))) {
> +    while ((hwi = audio_pcm_hw_find_any_in(s, hwi))) {
>          if (hwi->enabled) {
>              hwi->pcm_ops->ctl_in (hwi, VOICE_DISABLE);
>          }
> @@ -1459,7 +1457,7 @@ CaptureVoiceOut *AUD_add_capture (
>      cb->ops = *ops;
>      cb->opaque = cb_opaque;
>
> -    cap = audio_pcm_capture_find_specific (as);
> +    cap = audio_pcm_capture_find_specific(s, as);
>      if (cap) {
>          QLIST_INSERT_HEAD (&cap->cb_head, cb, entries);
>          return cap;
> @@ -1476,6 +1474,7 @@ CaptureVoiceOut *AUD_add_capture (
>          }
>
>          hw = &cap->hw;
> +        hw->s = s;
>          QLIST_INIT (&hw->sw_head);
>          QLIST_INIT (&cap->cb_head);
>
> @@ -1509,7 +1508,7 @@ CaptureVoiceOut *AUD_add_capture (
>          QLIST_INSERT_HEAD (&cap->cb_head, cb, entries);
>
>          hw = NULL;
> -        while ((hw = audio_pcm_hw_find_any_out (hw))) {
> +        while ((hw = audio_pcm_hw_find_any_out(s, hw))) {
>              audio_attach_capture (hw);
>          }
>          return cap;
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 59b2362..101081b 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -51,6 +51,7 @@ struct audio_pcm_info {
>  typedef struct SWVoiceCap SWVoiceCap;
>
>  typedef struct HWVoiceOut {
> +    AudioState *s;
>      int enabled;
>      int poll_mode;
>      int pending_disable;
> @@ -72,6 +73,7 @@ typedef struct HWVoiceOut {
>  } HWVoiceOut;
>
>  typedef struct HWVoiceIn {
> +    AudioState *s;
>      int enabled;
>      int poll_mode;
>      struct audio_pcm_info info;
> @@ -93,6 +95,7 @@ typedef struct HWVoiceIn {
>
>  struct SWVoiceOut {
>      QEMUSoundCard *card;
> +    AudioState *s;
>      struct audio_pcm_info info;
>      t_sample *conv;
>      int64_t ratio;
> @@ -110,6 +113,7 @@ struct SWVoiceOut {
>
>  struct SWVoiceIn {
>      QEMUSoundCard *card;
> +    AudioState *s;
>      int active;
>      struct audio_pcm_info info;
>      int64_t ratio;
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 096b2b3..455e50d 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -36,9 +36,9 @@
>  #define HWBUF hw->conv_buf
>  #endif
>
> -static void glue (audio_init_nb_voices_, TYPE) (struct audio_driver *drv)
> +static void glue(audio_init_nb_voices_, TYPE)(AudioState *s,
> +                                              struct audio_driver *drv)
>  {
> -    AudioState *s = &glob_audio_state;
>      int max_voices = glue (drv->max_voices_, TYPE);
>      int voice_size = glue (drv->voice_size_, TYPE);
>
> @@ -183,8 +183,8 @@ static void glue (audio_pcm_hw_del_sw_, TYPE) (SW *sw)
>
>  static void glue (audio_pcm_hw_gc_, TYPE) (HW **hwp)
>  {
> -    AudioState *s = &glob_audio_state;
>      HW *hw = *hwp;
> +    AudioState *s = hw->s;
>
>      if (!hw->sw_head.lh_first) {
>  #ifdef DAC
> @@ -199,15 +199,14 @@ static void glue (audio_pcm_hw_gc_, TYPE) (HW **hwp)
>      }
>  }
>
> -static HW *glue (audio_pcm_hw_find_any_, TYPE) (HW *hw)
> +static HW *glue(audio_pcm_hw_find_any_, TYPE)(AudioState *s, HW *hw)
>  {
> -    AudioState *s = &glob_audio_state;
>      return hw ? hw->entries.le_next : glue (s->hw_head_, TYPE).lh_first;
>  }
>
> -static HW *glue (audio_pcm_hw_find_any_enabled_, TYPE) (HW *hw)
> +static HW *glue(audio_pcm_hw_find_any_enabled_, TYPE)(AudioState *s, HW *hw)
>  {
> -    while ((hw = glue (audio_pcm_hw_find_any_, TYPE) (hw))) {
> +    while ((hw = glue(audio_pcm_hw_find_any_, TYPE)(s, hw))) {
>          if (hw->enabled) {
>              return hw;
>          }
> @@ -215,12 +214,10 @@ static HW *glue (audio_pcm_hw_find_any_enabled_, TYPE) 
> (HW *hw)
>      return NULL;
>  }
>
> -static HW *glue (audio_pcm_hw_find_specific_, TYPE) (
> -    HW *hw,
> -    struct audsettings *as
> -    )
> +static HW *glue(audio_pcm_hw_find_specific_, TYPE)(AudioState *s, HW *hw,
> +                                                   struct audsettings *as)
>  {
> -    while ((hw = glue (audio_pcm_hw_find_any_, TYPE) (hw))) {
> +    while ((hw = glue(audio_pcm_hw_find_any_, TYPE)(s, hw))) {
>          if (audio_pcm_info_eq (&hw->info, as)) {
>              return hw;
>          }
> @@ -228,10 +225,10 @@ static HW *glue (audio_pcm_hw_find_specific_, TYPE) (
>      return NULL;
>  }
>
> -static HW *glue (audio_pcm_hw_add_new_, TYPE) (struct audsettings *as)
> +static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
> +                                             struct audsettings *as)
>  {
>      HW *hw;
> -    AudioState *s = &glob_audio_state;
>      struct audio_driver *drv = s->drv;
>
>      if (!glue (s->nb_hw_voices_, TYPE)) {
> @@ -255,6 +252,7 @@ static HW *glue (audio_pcm_hw_add_new_, TYPE) (struct 
> audsettings *as)
>          return NULL;
>      }
>
> +    hw->s = s;
>      hw->pcm_ops = drv->pcm_ops;
>      hw->ctl_caps = drv->ctl_caps;
>
> @@ -299,33 +297,33 @@ static HW *glue (audio_pcm_hw_add_new_, TYPE) (struct 
> audsettings *as)
>      return NULL;
>  }
>
> -static HW *glue (audio_pcm_hw_add_, TYPE) (struct audsettings *as)
> +static HW *glue(audio_pcm_hw_add_, TYPE)(AudioState *s, struct audsettings 
> *as)
>  {
>      HW *hw;
> -    AudioState *s = &glob_audio_state;
>      AudiodevPerDirectionOptions *pdo = s->dev->TYPE;
>
>      if (pdo->fixed_settings) {
> -        hw = glue (audio_pcm_hw_add_new_, TYPE) (as);
> +        hw = glue(audio_pcm_hw_add_new_, TYPE)(s, as);
>          if (hw) {
>              return hw;
>          }
>      }
>
> -    hw = glue (audio_pcm_hw_find_specific_, TYPE) (NULL, as);
> +    hw = glue(audio_pcm_hw_find_specific_, TYPE)(s, NULL, as);
>      if (hw) {
>          return hw;
>      }
>
> -    hw = glue (audio_pcm_hw_add_new_, TYPE) (as);
> +    hw = glue(audio_pcm_hw_add_new_, TYPE)(s, as);
>      if (hw) {
>          return hw;
>      }
>
> -    return glue (audio_pcm_hw_find_any_, TYPE) (NULL);
> +    return glue(audio_pcm_hw_find_any_, TYPE)(s, NULL);
>  }
>
> -static SW *glue (audio_pcm_create_voice_pair_, TYPE) (
> +static SW *glue(audio_pcm_create_voice_pair_, TYPE)(
> +    AudioState *s,
>      const char *sw_name,
>      struct audsettings *as
>      )
> @@ -333,7 +331,6 @@ static SW *glue (audio_pcm_create_voice_pair_, TYPE) (
>      SW *sw;
>      HW *hw;
>      struct audsettings hw_as;
> -    AudioState *s = &glob_audio_state;
>      AudiodevPerDirectionOptions *pdo = s->dev->TYPE;
>
>      if (pdo->fixed_settings) {
> @@ -349,8 +346,9 @@ static SW *glue (audio_pcm_create_voice_pair_, TYPE) (
>                 sw_name ? sw_name : "unknown", sizeof (*sw));
>          goto err1;
>      }
> +    sw->s = s;
>
> -    hw = glue (audio_pcm_hw_add_, TYPE) (&hw_as);
> +    hw = glue(audio_pcm_hw_add_, TYPE)(s, &hw_as);
>      if (!hw) {
>          goto err2;
>      }
> @@ -447,7 +445,7 @@ SW *glue (AUD_open_, TYPE) (
>          }
>      }
>      else {
> -        sw = glue (audio_pcm_create_voice_pair_, TYPE) (name, as);
> +        sw = glue(audio_pcm_create_voice_pair_, TYPE)(s, name, as);
>          if (!sw) {
>              dolog ("Failed to create voice `%s'\n", name);
>              return NULL;
> --
> 2.4.5
>
>



-- 
Marc-André Lureau