Re: [Qemu-devel] [PATCH 02/11] audio: basic support for multi backend audio

2019-07-11 Thread Zoltán Kővágó
On 2019-07-11 16:37, Markus Armbruster wrote:
> "Zoltán Kővágó"  writes:
> 
>> On 2019-07-10 06:06, Markus Armbruster wrote:
>>> "Kővágó, Zoltán"  writes:
>>>
 Audio functions no longer access glob_audio_state, instead they get an
 AudioState as a parameter.  This is required in order to support
 multiple backends.

 glob_audio_state is also gone, and replaced with a tailq so we can store
 more than one states.

 Signed-off-by: Kővágó, Zoltán 
 ---
>>> [...]
 diff --git a/hmp-commands.hx b/hmp-commands.hx
 index bfa5681dd2..23196da3fe 100644
 --- a/hmp-commands.hx
 +++ b/hmp-commands.hx
 @@ -819,16 +819,17 @@ ETEXI
  
  {
  .name   = "wavcapture",
 -.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?",
 -.params = "path [frequency [bits [channels]]]",
 +.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?,audiodev:s?",
 +.params = "path [frequency [bits [channels [audiodev",
  .help   = "capture audio to a wave file (default 
 frequency=44100 bits=16 channels=2)",
  .cmd= hmp_wavcapture,
  },
  STEXI
 -@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
 [@var{channels}]]]
 +@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
 [@var{channels} [@var{audiodev}
  @findex wavcapture
 -Capture audio into @var{filename}. Using sample rate @var{frequency}
 -bits per sample @var{bits} and number of channels @var{channels}.
 +Capture audio into @var{filename} from @var{audiodev}. Using sample rate
 +@var{frequency} bits per sample @var{bits} and number of channels
 +@var{channels}.
  
  Defaults:
  @itemize @minus
>>>@item Sample rate = 44100 Hz - CD quality
>>>@item Bits = 16
>>>@item Number of channels = 2 - Stereo
>>>@end itemize
>>>ETEXI
>>>
>>> Defaults for the other optional arguments are listed here.  Why not for
>>> @audiodev?
>>
>> There's no default listed because there's no default when you use the
>> -audiodev options, since there's no good default.  When you don't use
>> -audiodev, it'll use the implicitly created audiodev which doesn't have
>> a name, so it can't be specified.
> 
> Double-checking to avoid misunderstandings: there is a default
> *behavior*, but no default *value*, i.e. there is no VALUE that makes
> audiodev=VALUE give you the same behavior as no audiodev.  Correct?

Yes.  If there is no audiodev=VALUE, and no -audiodev on the command
line, use the legacy config.  If there is audiodev=VALUE and -audiodev
id=VALUE, use that device.  Otherwise, it's an error.

> 
>>But I agree that this situation
>> should be documented somehow.
> 
> Yes, please.
> 
 diff --git a/qemu-options.hx b/qemu-options.hx
 index 9621e934c0..0111055aa4 100644
 --- a/qemu-options.hx
 +++ b/qemu-options.hx
 @@ -1978,6 +1978,11 @@ can help the device and guest to keep up and not 
 lose events in case
  events are arriving in bulk.  Possible causes for the latter are flaky
  network connections, or scripts for automated testing.
  
 +@item audiodev=@var{audiodev}
 +
 +Use the specified @var{audiodev} when the VNC client requests audio
 +transmission.
 +
>>>
>>> What's the default?
>>
>> It's the same story as wav_capture.
>>
>> Regards,
>> Zoltan




Re: [Qemu-devel] [PATCH 02/11] audio: basic support for multi backend audio

2019-07-11 Thread Markus Armbruster
"Zoltán Kővágó"  writes:

> On 2019-07-10 06:06, Markus Armbruster wrote:
>> "Kővágó, Zoltán"  writes:
>> 
>>> Audio functions no longer access glob_audio_state, instead they get an
>>> AudioState as a parameter.  This is required in order to support
>>> multiple backends.
>>>
>>> glob_audio_state is also gone, and replaced with a tailq so we can store
>>> more than one states.
>>>
>>> Signed-off-by: Kővágó, Zoltán 
>>> ---
>> [...]
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index bfa5681dd2..23196da3fe 100644
>>> --- a/hmp-commands.hx
>>> +++ b/hmp-commands.hx
>>> @@ -819,16 +819,17 @@ ETEXI
>>>  
>>>  {
>>>  .name   = "wavcapture",
>>> -.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?",
>>> -.params = "path [frequency [bits [channels]]]",
>>> +.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?,audiodev:s?",
>>> +.params = "path [frequency [bits [channels [audiodev",
>>>  .help   = "capture audio to a wave file (default 
>>> frequency=44100 bits=16 channels=2)",
>>>  .cmd= hmp_wavcapture,
>>>  },
>>>  STEXI
>>> -@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
>>> [@var{channels}]]]
>>> +@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
>>> [@var{channels} [@var{audiodev}
>>>  @findex wavcapture
>>> -Capture audio into @var{filename}. Using sample rate @var{frequency}
>>> -bits per sample @var{bits} and number of channels @var{channels}.
>>> +Capture audio into @var{filename} from @var{audiodev}. Using sample rate
>>> +@var{frequency} bits per sample @var{bits} and number of channels
>>> +@var{channels}.
>>>  
>>>  Defaults:
>>>  @itemize @minus
>>@item Sample rate = 44100 Hz - CD quality
>>@item Bits = 16
>>@item Number of channels = 2 - Stereo
>>@end itemize
>>ETEXI
>> 
>> Defaults for the other optional arguments are listed here.  Why not for
>> @audiodev?
>
> There's no default listed because there's no default when you use the
> -audiodev options, since there's no good default.  When you don't use
> -audiodev, it'll use the implicitly created audiodev which doesn't have
> a name, so it can't be specified.

Double-checking to avoid misunderstandings: there is a default
*behavior*, but no default *value*, i.e. there is no VALUE that makes
audiodev=VALUE give you the same behavior as no audiodev.  Correct?

>But I agree that this situation
> should be documented somehow.

Yes, please.

>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 9621e934c0..0111055aa4 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -1978,6 +1978,11 @@ can help the device and guest to keep up and not 
>>> lose events in case
>>>  events are arriving in bulk.  Possible causes for the latter are flaky
>>>  network connections, or scripts for automated testing.
>>>  
>>> +@item audiodev=@var{audiodev}
>>> +
>>> +Use the specified @var{audiodev} when the VNC client requests audio
>>> +transmission.
>>> +
>> 
>> What's the default?
>
> It's the same story as wav_capture.
>
> Regards,
> Zoltan



Re: [Qemu-devel] [PATCH 02/11] audio: basic support for multi backend audio

2019-07-11 Thread Dr. David Alan Gilbert
* Zoltán Kővágó (dirty.ice...@gmail.com) wrote:
> On 2019-07-10 06:06, Markus Armbruster wrote:
> > "Kővágó, Zoltán"  writes:
> > 
> >> Audio functions no longer access glob_audio_state, instead they get an
> >> AudioState as a parameter.  This is required in order to support
> >> multiple backends.
> >>
> >> glob_audio_state is also gone, and replaced with a tailq so we can store
> >> more than one states.
> >>
> >> Signed-off-by: Kővágó, Zoltán 
> >> ---
> > [...]
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index bfa5681dd2..23196da3fe 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -819,16 +819,17 @@ ETEXI
> >>  
> >>  {
> >>  .name   = "wavcapture",
> >> -.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?",
> >> -.params = "path [frequency [bits [channels]]]",
> >> +.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?,audiodev:s?",
> >> +.params = "path [frequency [bits [channels [audiodev",
> >>  .help   = "capture audio to a wave file (default 
> >> frequency=44100 bits=16 channels=2)",
> >>  .cmd= hmp_wavcapture,
> >>  },
> >>  STEXI
> >> -@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
> >> [@var{channels}]]]
> >> +@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
> >> [@var{channels} [@var{audiodev}
> >>  @findex wavcapture
> >> -Capture audio into @var{filename}. Using sample rate @var{frequency}
> >> -bits per sample @var{bits} and number of channels @var{channels}.
> >> +Capture audio into @var{filename} from @var{audiodev}. Using sample rate
> >> +@var{frequency} bits per sample @var{bits} and number of channels
> >> +@var{channels}.
> >>  
> >>  Defaults:
> >>  @itemize @minus
> >@item Sample rate = 44100 Hz - CD quality
> >@item Bits = 16
> >@item Number of channels = 2 - Stereo
> >@end itemize
> >ETEXI
> > 
> > Defaults for the other optional arguments are listed here.  Why not for
> > @audiodev?
> 
> There's no default listed because there's no default when you use the
> -audiodev options, since there's no good default.  When you don't use
> -audiodev, it'll use the implicitly created audiodev which doesn't have
> a name, so it can't be specified.  But I agree that this situation
> should be documented somehow.
> 
> > 
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index 9621e934c0..0111055aa4 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -1978,6 +1978,11 @@ can help the device and guest to keep up and not 
> >> lose events in case
> >>  events are arriving in bulk.  Possible causes for the latter are flaky
> >>  network connections, or scripts for automated testing.
> >>  
> >> +@item audiodev=@var{audiodev}
> >> +
> >> +Use the specified @var{audiodev} when the VNC client requests audio
> >> +transmission.
> >> +
> > 
> > What's the default?
> 
> It's the same story as wav_capture.

OK, so I think you're saying the default behaviour doesn't change.
Add a comment to document that, and that seems OK.

Dave

> Regards,
> Zoltan
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH 02/11] audio: basic support for multi backend audio

2019-07-10 Thread Marc-André Lureau
On Tue, Jul 9, 2019 at 11:02 PM Kővágó, Zoltán  wrote:
>
> Audio functions no longer access glob_audio_state, instead they get an
> AudioState as a parameter.  This is required in order to support
> multiple backends.
>
> glob_audio_state is also gone, and replaced with a tailq so we can store
> more than one states.
>
> Signed-off-by: Kővágó, Zoltán 


I guess you could have made the vnc and wavcapture audiodev argument a
seperate commit. Or mention it in commit message.

Reviewed-by: Marc-André Lureau 




> ---
>  audio/audio.h  |  12 +++--
>  audio/audio_int.h  |   2 +
>  audio/audio_template.h |   2 +-
>  ui/vnc.h   |   2 +
>  audio/audio.c  | 102 +++--
>  audio/wavcapture.c |   6 +--
>  monitor/misc.c |  12 -
>  ui/vnc.c   |  15 +-
>  hmp-commands.hx|  11 +++--
>  qemu-options.hx|   5 ++
>  10 files changed, 131 insertions(+), 38 deletions(-)
>
> diff --git a/audio/audio.h b/audio/audio.h
> index 64b0f761bc..ad2457f4de 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -78,8 +78,10 @@ typedef struct SWVoiceOut SWVoiceOut;
>  typedef struct CaptureVoiceOut CaptureVoiceOut;
>  typedef struct SWVoiceIn SWVoiceIn;
>
> +typedef struct AudioState AudioState;
>  typedef struct QEMUSoundCard {
>  char *name;
> +AudioState *state;
>  QLIST_ENTRY (QEMUSoundCard) entries;
>  } QEMUSoundCard;
>
> @@ -92,7 +94,8 @@ void AUD_log (const char *cap, const char *fmt, ...) 
> GCC_FMT_ATTR(2, 3);
>
>  void AUD_register_card (const char *name, QEMUSoundCard *card);
>  void AUD_remove_card (QEMUSoundCard *card);
> -CaptureVoiceOut *AUD_add_capture (
> +CaptureVoiceOut *AUD_add_capture(
> +AudioState *s,
>  struct audsettings *as,
>  struct audio_capture_ops *ops,
>  void *opaque
> @@ -160,8 +163,8 @@ static inline void *advance (void *p, int incr)
>  #define audio_MAX(a, b) ((a)<(b)?(b):(a))
>  #endif
>
> -int wav_start_capture (CaptureState *s, const char *path, int freq,
> -   int bits, int nchannels);
> +int wav_start_capture(AudioState *state, CaptureState *s, const char *path,
> +  int freq, int bits, int nchannels);
>
>  bool audio_is_cleaning_up(void);
>  void audio_cleanup(void);
> @@ -175,4 +178,7 @@ void audio_parse_option(const char *opt);
>  void audio_init_audiodevs(void);
>  void audio_legacy_help(void);
>
> +AudioState *audio_state_by_name(const char *name);
> +const char *audio_get_id(QEMUSoundCard *card);
> +
>  #endif /* QEMU_AUDIO_H */
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 8164696b2c..9f01f6ad00 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -196,6 +196,8 @@ typedef struct AudioState {
>
>  bool timer_running;
>  uint64_t timer_last;
> +
> +QTAILQ_ENTRY(AudioState) list;
>  } AudioState;
>
>  extern const struct mixeng_volume nominal_volume;
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index c721fed75d..54f07338e7 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -428,7 +428,7 @@ SW *glue (AUD_open_, TYPE) (
>  struct audsettings *as
>  )
>  {
> -AudioState *s = &glob_audio_state;
> +AudioState *s = card->state;
>  AudiodevPerDirectionOptions *pdo = glue(audio_get_pdo_, TYPE)(s->dev);
>
>  if (audio_bug(__func__, !card || !name || !callback_fn || !as)) {
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 2f84db3142..6f54653455 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -183,6 +183,8 @@ struct VncDisplay
>  #ifdef CONFIG_VNC_SASL
>  VncDisplaySASL sasl;
>  #endif
> +
> +AudioState *audio_state;
>  };
>
>  typedef struct VncTight {
> diff --git a/audio/audio.c b/audio/audio.c
> index 8d2f580788..e9dd7c8b32 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -87,7 +87,8 @@ audio_driver *audio_driver_lookup(const char *name)
>  return NULL;
>  }
>
> -static AudioState glob_audio_state;
> +static QTAILQ_HEAD(AudioStateHead, AudioState) audio_states =
> +QTAILQ_HEAD_INITIALIZER(audio_states);
>
>  const struct mixeng_volume nominal_volume = {
>  .mute = 0,
> @@ -1236,11 +1237,14 @@ static void audio_run_capture (AudioState *s)
>
>  void audio_run (const char *msg)
>  {
> -AudioState *s = &glob_audio_state;
> +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
>  {
>  static double prevtime;
> @@ -1304,13 +1308,11 @@ bool audio_is_cleaning_up(void)
>  return is_cleaning_up;
>  }
>
> -void audio_cleanup(void)
> +static void free_audio_state(AudioState *s)
>  {
> -AudioState *s = &glob_audio_state;
>  HWVoiceOut *hwo, *hwon;
>  HWVoiceIn *hwi, *hwin;
>
> -is_cleaning_up = true;
>  QLIST_FOREACH_SAFE(hwo, &s->hw_head_out, entries, hw

Re: [Qemu-devel] [PATCH 02/11] audio: basic support for multi backend audio

2019-07-10 Thread Zoltán Kővágó
On 2019-07-10 06:06, Markus Armbruster wrote:
> "Kővágó, Zoltán"  writes:
> 
>> Audio functions no longer access glob_audio_state, instead they get an
>> AudioState as a parameter.  This is required in order to support
>> multiple backends.
>>
>> glob_audio_state is also gone, and replaced with a tailq so we can store
>> more than one states.
>>
>> Signed-off-by: Kővágó, Zoltán 
>> ---
> [...]
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index bfa5681dd2..23196da3fe 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -819,16 +819,17 @@ ETEXI
>>  
>>  {
>>  .name   = "wavcapture",
>> -.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?",
>> -.params = "path [frequency [bits [channels]]]",
>> +.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?,audiodev:s?",
>> +.params = "path [frequency [bits [channels [audiodev",
>>  .help   = "capture audio to a wave file (default 
>> frequency=44100 bits=16 channels=2)",
>>  .cmd= hmp_wavcapture,
>>  },
>>  STEXI
>> -@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
>> [@var{channels}]]]
>> +@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
>> [@var{channels} [@var{audiodev}
>>  @findex wavcapture
>> -Capture audio into @var{filename}. Using sample rate @var{frequency}
>> -bits per sample @var{bits} and number of channels @var{channels}.
>> +Capture audio into @var{filename} from @var{audiodev}. Using sample rate
>> +@var{frequency} bits per sample @var{bits} and number of channels
>> +@var{channels}.
>>  
>>  Defaults:
>>  @itemize @minus
>@item Sample rate = 44100 Hz - CD quality
>@item Bits = 16
>@item Number of channels = 2 - Stereo
>@end itemize
>ETEXI
> 
> Defaults for the other optional arguments are listed here.  Why not for
> @audiodev?

There's no default listed because there's no default when you use the
-audiodev options, since there's no good default.  When you don't use
-audiodev, it'll use the implicitly created audiodev which doesn't have
a name, so it can't be specified.  But I agree that this situation
should be documented somehow.

> 
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 9621e934c0..0111055aa4 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1978,6 +1978,11 @@ can help the device and guest to keep up and not lose 
>> events in case
>>  events are arriving in bulk.  Possible causes for the latter are flaky
>>  network connections, or scripts for automated testing.
>>  
>> +@item audiodev=@var{audiodev}
>> +
>> +Use the specified @var{audiodev} when the VNC client requests audio
>> +transmission.
>> +
> 
> What's the default?

It's the same story as wav_capture.

Regards,
Zoltan



Re: [Qemu-devel] [PATCH 02/11] audio: basic support for multi backend audio

2019-07-09 Thread Markus Armbruster
"Kővágó, Zoltán"  writes:

> Audio functions no longer access glob_audio_state, instead they get an
> AudioState as a parameter.  This is required in order to support
> multiple backends.
>
> glob_audio_state is also gone, and replaced with a tailq so we can store
> more than one states.
>
> Signed-off-by: Kővágó, Zoltán 
> ---
[...]
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index bfa5681dd2..23196da3fe 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -819,16 +819,17 @@ ETEXI
>  
>  {
>  .name   = "wavcapture",
> -.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?",
> -.params = "path [frequency [bits [channels]]]",
> +.args_type  = "path:F,freq:i?,bits:i?,nchannels:i?,audiodev:s?",
> +.params = "path [frequency [bits [channels [audiodev",
>  .help   = "capture audio to a wave file (default frequency=44100 
> bits=16 channels=2)",
>  .cmd= hmp_wavcapture,
>  },
>  STEXI
> -@item wavcapture @var{filename} [@var{frequency} [@var{bits} 
> [@var{channels}]]]
> +@item wavcapture @var{filename} [@var{frequency} [@var{bits} [@var{channels} 
> [@var{audiodev}
>  @findex wavcapture
> -Capture audio into @var{filename}. Using sample rate @var{frequency}
> -bits per sample @var{bits} and number of channels @var{channels}.
> +Capture audio into @var{filename} from @var{audiodev}. Using sample rate
> +@var{frequency} bits per sample @var{bits} and number of channels
> +@var{channels}.
>  
>  Defaults:
>  @itemize @minus
   @item Sample rate = 44100 Hz - CD quality
   @item Bits = 16
   @item Number of channels = 2 - Stereo
   @end itemize
   ETEXI

Defaults for the other optional arguments are listed here.  Why not for
@audiodev?

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9621e934c0..0111055aa4 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1978,6 +1978,11 @@ can help the device and guest to keep up and not lose 
> events in case
>  events are arriving in bulk.  Possible causes for the latter are flaky
>  network connections, or scripts for automated testing.
>  
> +@item audiodev=@var{audiodev}
> +
> +Use the specified @var{audiodev} when the VNC client requests audio
> +transmission.
> +

What's the default?

>  @end table
>  ETEXI