[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 06/52] audio: -audiodev command line option b
From: |
Zoltán Kővágó |
Subject: |
Re: [Qemu-devel] [PATCH v2 06/52] audio: -audiodev command line option basic implementation |
Date: |
Mon, 7 Jan 2019 21:48:06 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2019-01-07 14:13, Markus Armbruster wrote:
> "Kővágó, Zoltán" <address@hidden> 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 <address@hidden>
>> ---
> [...]
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 96cbd57c37..e7f25ea84b 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
> [...]
>> @@ -2127,3 +1841,158 @@ void AUD_set_volume_in (SWVoiceIn *sw, int mute,
>> uint8_t lvol, uint8_t rvol)
>> }
>> }
>> }
>> +
>> +QemuOptsList qemu_audiodev_opts = {
>> + .name = "audiodev",
>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_audiodev_opts.head),
>> + .implied_opt_name = "driver",
>> + .desc = {
>> + /*
>> + * no elements => accept any params
>> + * sanity checking will happen later
>> + */
>> + { /* end of list */ }
>> + },
>> +};
>> +
>> +static void validate_per_direction_opts(AudiodevPerDirectionOptions *pdo,
>> + Error **errp)
>> +{
>> + 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 Audiodev *parse_option(QemuOpts *opts, Error **errp)
>> +{
>> + Error *local_err = NULL;
>> + Visitor *v = opts_visitor_new(opts, true);
>> + Audiodev *dev = NULL;
>> + visit_type_Audiodev(v, NULL, &dev, &local_err);
>> + visit_free(v);
>> +
>> + if (local_err) {
>> + goto err2;
>> + }
>> +
>> + validate_per_direction_opts(dev->in, &local_err);
>> + if (local_err) {
>> + goto err;
>> + }
>> + validate_per_direction_opts(dev->out, &local_err);
>> + if (local_err) {
>> + goto err;
>> + }
>> +
>> + if (!dev->has_timer_period) {
>> + dev->has_timer_period = true;
>> + dev->timer_period = 10000; /* 100Hz -> 10ms */
>> + }
>> +
>> + return dev;
>> +
>> +err:
>> + qapi_free_Audiodev(dev);
>> +err2:
>> + error_propagate(errp, local_err);
>> + return NULL;
>> +}
>> +
>> +static int each_option(void *opaque, QemuOpts *opts, Error **errp)
>> +{
>> + Audiodev *dev = parse_option(opts, errp);
>> + if (!dev) {
>> + return -1;
>> + }
>> + return audio_init(dev);
>> +}
>> +
>> +void audio_set_options(void)
>> +{
>> + if (qemu_opts_foreach(qemu_find_opts("audiodev"), each_option, NULL,
>> + &error_abort)) {
>> + exit(1);
>> + }
>> +}
> [...]
>> diff --git a/vl.c b/vl.c
>> index 8353d3c718..b5364ffe46 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3074,6 +3074,7 @@ int main(int argc, char **argv, char **envp)
>> qemu_add_opts(&qemu_option_rom_opts);
>> qemu_add_opts(&qemu_machine_opts);
>> qemu_add_opts(&qemu_accel_opts);
>> + qemu_add_opts(&qemu_audiodev_opts);
>> qemu_add_opts(&qemu_mem_opts);
>> qemu_add_opts(&qemu_smp_opts);
>> qemu_add_opts(&qemu_boot_opts);
>> @@ -3307,9 +3308,15 @@ int main(int argc, char **argv, char **envp)
>> add_device_config(DEV_BT, optarg);
>> break;
>> case QEMU_OPTION_audio_help:
>> - AUD_help ();
>> + audio_legacy_help();
>> exit (0);
>> break;
>> + case QEMU_OPTION_audiodev:
>> + if (!qemu_opts_parse_noisily(qemu_find_opts("audiodev"),
>> + optarg, true)) {
>> + exit(1);
>> + }
>> + break;
>> case QEMU_OPTION_soundhw:
>> select_soundhw (optarg);
>> break;
>> @@ -4545,6 +4552,8 @@ int main(int argc, char **argv, char **envp)
>> /* do monitor/qmp handling at preconfig state if requested */
>> main_loop();
>>
>> + audio_set_options();
>> +
>> /* from here on runstate is RUN_STATE_PRELAUNCH */
>> machine_run_board_init(current_machine);
>
> This uses the options visitor, roughly like -netdev. Depends on your
> options visitor enhancements. Is there any particular reason not to do
> it like -blockdev and -display, with qobject_input_visitor_new_str()?
> I'm asking because I'd very much like new code to use
> qobject_input_visitor_new_str() rather than the options visitor.
>
> Today, the options visitor looks like an evolutionary dead end. My
> command-line qapification (still stuck at the RFC stage, hope to get it
> unstuck this year) relies on qobject_input_visitor_new_str(). The goal
> is to convert *all* of the command line to it.
>
> I suspect your series uses the options visitor only because back when
> you started, qobject_input_visitor_new_str() didn't exist.
>
Yes, this patch series is a bit old, and at that time this was the best
I could do. I can look into it this (probably only on the weekend
though), it looks like it supports foo.bar=something syntax, so I don't
have to specify a json on the command line...
Regards,
Zoltan
Re: [Qemu-devel] [PATCH v2 06/52] audio: -audiodev command line option basic implementation, Gerd Hoffmann, 2019/01/08