Re: [Qemu-devel] [PATCH v4 04/14] audio: -audiodev command line option basic implementation
On 2019-01-29 16:56, Markus Armbruster wrote:
> "Kővágó, Zoltán" writes:
>
>> Audio drivers now get an Audiodev * as config paramters, instead of the
>> global audio_option structs. There is some code in audio/audio_legacy.c
>> that converts the old environment variables to audiodev options (this
>> way backends do not have to worry about legacy options). It also
>> contains a replacement of -audio-help, which prints out the equivalent
>> -audiodev based config of the currently specified environment variables.
>>
>> Note that backends are not updated and still rely on environment
>> variables.
>>
>> Also note that (due to moving try-poll from global to backend specific
>> option) currently ALSA and OSS will always try poll mode, regardless of
>> environment variables or -audiodev options.
>>
>> Signed-off-by: Kővágó, Zoltán
>> ---
> [...]
>> diff --git a/audio/audio.c b/audio/audio.c
>> index ce8e6ea8c2..b37c245a8a 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
> [...]
>> @@ -2129,3 +1866,145 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute,
>> uint8_t lvol, uint8_t rvol)
>> }
>> }
>> }
>> +
>> +static void audio_validate_per_direction_opts(
>> +AudiodevPerDirectionOptions **pdo, bool *has_pdo, Error **errp)
>> +{
>> +if (!*has_pdo) {
>> +*pdo = g_malloc0(sizeof(AudiodevPerDirectionOptions));
>> +*has_pdo = true;
>> +}
>> +
>> +if (!(*pdo)->has_fixed_settings) {
>> +(*pdo)->has_fixed_settings = true;
>> +(*pdo)->fixed_settings = true;
>> +}
>> +if (!(*pdo)->fixed_settings &&
>> +((*pdo)->has_frequency || (*pdo)->has_channels ||
>> (*pdo)->has_format)) {
>> +error_setg(errp,
>> + "You can't use frequency, channels or format with
>> fixed-settings=off");
>> +return;
>> +}
>> +
>> +if (!(*pdo)->has_frequency) {
>> +(*pdo)->has_frequency = true;
>> +(*pdo)->frequency = 44100;
>> +}
>> +if (!(*pdo)->has_channels) {
>> +(*pdo)->has_channels = true;
>> +(*pdo)->channels = 2;
>> +}
>> +if (!(*pdo)->has_voices) {
>> +(*pdo)->has_voices = true;
>> +(*pdo)->voices = 1;
>> +}
>> +if (!(*pdo)->has_format) {
>> +(*pdo)->has_format = true;
>> +(*pdo)->format = AUDIO_FORMAT_S16;
>> +}
>> +}
>> +
>> +static void audio_validate_opts(Audiodev *dev, Error **errp)
>> +{
>> +Error *err = NULL;
>> +
>> +audio_validate_per_direction_opts(>in, >has_in, );
>> +if (err) {
>> +error_propagate(errp, err);
>> +return;
>> +}
>> +
>> +audio_validate_per_direction_opts(>out, >has_out, );
>> +if (err) {
>> +error_propagate(errp, err);
>> +return;
>> +}
>> +
>> +if (!dev->has_timer_period) {
>> +dev->has_timer_period = true;
>> +dev->timer_period = 1; /* 100Hz -> 10ms */
>> +}
>> +}
>
> Drive-by question: this validates just the generic part of Audiodev gets
> validated, not the driver-specific part. Is there nothing to validate?
> Does it happen somewhere else?
Driver specific validation/initialization happens in each driver's init
function (they're added in the upcoming patches).
>> +
>> +void audio_parse_option(const char *opt)
>> +{
>> +AudiodevListEntry *e;
>> +Audiodev *dev = NULL;
>> +
>> +Visitor *v = qobject_input_visitor_new_str(opt, "driver", _fatal);
>> +visit_type_Audiodev(v, NULL, , _fatal);
>> +visit_free(v);
>> +
>> +audio_validate_opts(dev, _fatal);
>> +
>> +e = g_malloc0(sizeof(AudiodevListEntry));
>> +e->dev = dev;
>> +QSIMPLEQ_INSERT_TAIL(, e, next);
>> +}
>> +
>> +void audio_init_audiodevs(void)
>> +{
>> +AudiodevListEntry *e;
>> +
>> +QSIMPLEQ_FOREACH(e, , next) {
>> +audio_init(e->dev);
>> +}
>> +}
>
> Your use of QAPI here looks good to me.
>
> [...]
>
Re: [Qemu-devel] [PATCH v4 04/14] audio: -audiodev command line option basic implementation
"Kővágó, Zoltán" writes:
> Audio drivers now get an Audiodev * as config paramters, instead of the
> global audio_option structs. There is some code in audio/audio_legacy.c
> that converts the old environment variables to audiodev options (this
> way backends do not have to worry about legacy options). It also
> contains a replacement of -audio-help, which prints out the equivalent
> -audiodev based config of the currently specified environment variables.
>
> Note that backends are not updated and still rely on environment
> variables.
>
> Also note that (due to moving try-poll from global to backend specific
> option) currently ALSA and OSS will always try poll mode, regardless of
> environment variables or -audiodev options.
>
> Signed-off-by: Kővágó, Zoltán
> ---
[...]
> diff --git a/audio/audio.c b/audio/audio.c
> index ce8e6ea8c2..b37c245a8a 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
[...]
> @@ -2129,3 +1866,145 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute,
> uint8_t lvol, uint8_t rvol)
> }
> }
> }
> +
> +static void audio_validate_per_direction_opts(
> +AudiodevPerDirectionOptions **pdo, bool *has_pdo, Error **errp)
> +{
> +if (!*has_pdo) {
> +*pdo = g_malloc0(sizeof(AudiodevPerDirectionOptions));
> +*has_pdo = true;
> +}
> +
> +if (!(*pdo)->has_fixed_settings) {
> +(*pdo)->has_fixed_settings = true;
> +(*pdo)->fixed_settings = true;
> +}
> +if (!(*pdo)->fixed_settings &&
> +((*pdo)->has_frequency || (*pdo)->has_channels ||
> (*pdo)->has_format)) {
> +error_setg(errp,
> + "You can't use frequency, channels or format with
> fixed-settings=off");
> +return;
> +}
> +
> +if (!(*pdo)->has_frequency) {
> +(*pdo)->has_frequency = true;
> +(*pdo)->frequency = 44100;
> +}
> +if (!(*pdo)->has_channels) {
> +(*pdo)->has_channels = true;
> +(*pdo)->channels = 2;
> +}
> +if (!(*pdo)->has_voices) {
> +(*pdo)->has_voices = true;
> +(*pdo)->voices = 1;
> +}
> +if (!(*pdo)->has_format) {
> +(*pdo)->has_format = true;
> +(*pdo)->format = AUDIO_FORMAT_S16;
> +}
> +}
> +
> +static void audio_validate_opts(Audiodev *dev, Error **errp)
> +{
> +Error *err = NULL;
> +
> +audio_validate_per_direction_opts(>in, >has_in, );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +
> +audio_validate_per_direction_opts(>out, >has_out, );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +
> +if (!dev->has_timer_period) {
> +dev->has_timer_period = true;
> +dev->timer_period = 1; /* 100Hz -> 10ms */
> +}
> +}
Drive-by question: this validates just the generic part of Audiodev gets
validated, not the driver-specific part. Is there nothing to validate?
Does it happen somewhere else?
> +
> +void audio_parse_option(const char *opt)
> +{
> +AudiodevListEntry *e;
> +Audiodev *dev = NULL;
> +
> +Visitor *v = qobject_input_visitor_new_str(opt, "driver", _fatal);
> +visit_type_Audiodev(v, NULL, , _fatal);
> +visit_free(v);
> +
> +audio_validate_opts(dev, _fatal);
> +
> +e = g_malloc0(sizeof(AudiodevListEntry));
> +e->dev = dev;
> +QSIMPLEQ_INSERT_TAIL(, e, next);
> +}
> +
> +void audio_init_audiodevs(void)
> +{
> +AudiodevListEntry *e;
> +
> +QSIMPLEQ_FOREACH(e, , next) {
> +audio_init(e->dev);
> +}
> +}
Your use of QAPI here looks good to me.
[...]
[Qemu-devel] [PATCH v4 04/14] audio: -audiodev command line option basic implementation
Audio drivers now get an Audiodev * as config paramters, instead of the
global audio_option structs. There is some code in audio/audio_legacy.c
that converts the old environment variables to audiodev options (this
way backends do not have to worry about legacy options). It also
contains a replacement of -audio-help, which prints out the equivalent
-audiodev based config of the currently specified environment variables.
Note that backends are not updated and still rely on environment
variables.
Also note that (due to moving try-poll from global to backend specific
option) currently ALSA and OSS will always try poll mode, regardless of
environment variables or -audiodev options.
Signed-off-by: Kővágó, Zoltán
---
Notes:
Changes from v2:
* MAJOR: use qobject_input_visitor instead of QemuOpts
* almost completely rewrote legacy options handling
* added missing license comment to audio_legacy.c0
audio/audio.h | 18 +-
audio/audio_int.h | 16 +-
audio/audio_template.h | 13 +-
audio/alsaaudio.c | 2 +-
audio/audio.c | 601 -
audio/audio_legacy.c | 295
audio/coreaudio.c | 2 +-
audio/dsoundaudio.c| 2 +-
audio/noaudio.c| 2 +-
audio/ossaudio.c | 2 +-
audio/paaudio.c| 2 +-
audio/sdlaudio.c | 2 +-
audio/spiceaudio.c | 2 +-
audio/wavaudio.c | 2 +-
vl.c | 7 +-
audio/Makefile.objs| 2 +-
16 files changed, 590 insertions(+), 380 deletions(-)
create mode 100644 audio/audio_legacy.c
diff --git a/audio/audio.h b/audio/audio.h
index 02f29a3b3e..64b0f761bc 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -36,12 +36,21 @@ typedef void (*audio_callback_fn) (void *opaque, int avail);
#define AUDIO_HOST_ENDIANNESS 0
#endif
-struct audsettings {
+typedef struct audsettings {
int freq;
int nchannels;
AudioFormat fmt;
int endianness;
-};
+} audsettings;
+
+audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo);
+int audioformat_bytes_per_sample(AudioFormat fmt);
+int audio_buffer_frames(AudiodevPerDirectionOptions *pdo,
+audsettings *as, int def_usecs);
+int audio_buffer_samples(AudiodevPerDirectionOptions *pdo,
+ audsettings *as, int def_usecs);
+int audio_buffer_bytes(AudiodevPerDirectionOptions *pdo,
+ audsettings *as, int def_usecs);
typedef enum {
AUD_CNOTIFY_ENABLE,
@@ -81,7 +90,6 @@ typedef struct QEMUAudioTimeStamp {
void AUD_vlog (const char *cap, const char *fmt, va_list ap) GCC_FMT_ATTR(2,
0);
void AUD_log (const char *cap, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
-void AUD_help (void);
void AUD_register_card (const char *name, QEMUSoundCard *card);
void AUD_remove_card (QEMUSoundCard *card);
CaptureVoiceOut *AUD_add_capture (
@@ -163,4 +171,8 @@ void audio_sample_to_uint64(void *samples, int pos,
void audio_sample_from_uint64(void *samples, int pos,
uint64_t left, uint64_t right);
+void audio_parse_option(const char *opt);
+void audio_init_audiodevs(void);
+void audio_legacy_help(void);
+
#endif /* QEMU_AUDIO_H */
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 6c451b995c..cee46c4809 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -146,7 +146,7 @@ struct audio_driver {
const char *name;
const char *descr;
struct audio_option *options;
-void *(*init) (void);
+void *(*init) (Audiodev *);
void (*fini) (void *);
struct audio_pcm_ops *pcm_ops;
int can_be_default;
@@ -193,6 +193,7 @@ struct SWVoiceCap {
typedef struct AudioState {
struct audio_driver *drv;
+Audiodev *dev;
void *drv_opaque;
QEMUTimer *ts;
@@ -203,10 +204,13 @@ typedef struct AudioState {
int nb_hw_voices_out;
int nb_hw_voices_in;
int vm_running;
+int64_t period_ticks;
} AudioState;
extern const struct mixeng_volume nominal_volume;
+extern const char *audio_prio_list[];
+
void audio_driver_register(audio_driver *drv);
audio_driver *audio_driver_lookup(const char *name);
@@ -248,4 +252,14 @@ static inline int audio_ring_dist (int dst, int src, int
len)
#define AUDIO_STRINGIFY_(n) #n
#define AUDIO_STRINGIFY(n) AUDIO_STRINGIFY_(n)
+typedef struct AudiodevListEntry {
+Audiodev *dev;
+QSIMPLEQ_ENTRY(AudiodevListEntry) next;
+} AudiodevListEntry;
+
+typedef QSIMPLEQ_HEAD(, AudiodevListEntry) AudiodevListHead;
+AudiodevListHead audio_handle_legacy_opts(void);
+
+void audio_free_audiodev_list(AudiodevListHead *head);
+
#endif /* QEMU_AUDIO_INT_H */
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 7de227d2d1..c1d7207abd 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -302,8 +302,10 @@ static HW *glue (audio_pcm_hw_add_new_, TYPE) (struct
audsettings *as)
static HW *glue (audio_pcm_hw_add_, TYPE) (struct audsettings *as)
{
HW
