| Message ID | 26b300d6c17e6ac015f1519d50151744cf806c03.1545598229.git.DirtY.iCE.hu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Audio 5.1 patches | expand |
"Kővágó, Zoltán" <dirty.ice.hu@gmail.com> 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 <DirtY.iCE.hu@gmail.com> > --- [...] > 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.
On 2019-01-07 14:13, Markus Armbruster wrote: > "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> 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 <DirtY.iCE.hu@gmail.com> >> --- > [...] >> 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
"Zoltán Kővágó" <dirty.ice.hu@gmail.com> writes: > On 2019-01-07 14:13, Markus Armbruster wrote: >> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> 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 <DirtY.iCE.hu@gmail.com> >>> --- >> [...] >>> 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... The dotted key syntax and how it relates to JSON is documented in util/keyval.c. If you have further questions on this stuff, please don't hesitate to ask me. If you get to the point where you feel your (beefy) series has gotten stuck on the conversion away from the options visitor, let's discuss how to best get it unstuck. Thanks!
Hi, > > 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... It supports both, i.e. this ... qemu -display gtk,full-screen=on ... and this ... qemu -display '{ "type": "gtk", "full-screen": true }' ... has the same effect. cheers, Gerd
On 2019-01-08 04:42, Markus Armbruster wrote: > "Zoltán Kővágó" <dirty.ice.hu@gmail.com> writes: > >> On 2019-01-07 14:13, Markus Armbruster wrote: >>> "Kővágó, Zoltán" <dirty.ice.hu@gmail.com> 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 <DirtY.iCE.hu@gmail.com> >>>> --- >>> [...] >>>> 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... > > The dotted key syntax and how it relates to JSON is documented in > util/keyval.c. > > If you have further questions on this stuff, please don't hesitate to > ask me. > > If you get to the point where you feel your (beefy) series has gotten > stuck on the conversion away from the options visitor, let's discuss how > to best get it unstuck. > > Thanks! > I briefly looked at converting this single patch, and of course I ran into a few issues. The audio_legacy code currently creates a QemuOpts from the environment variables. This had a nice effect that once I converted the options, I could print them with qemu_opts_print, helping the user how to convert the old env variables to -audiodev, since some of the conversions are not so trivial/straightforward. I converted them to QDict, which complicated things a bit since now I have to do proper nesting (until now I could just set key to "foo.bar" and it worked), especially in the *_to_usecs functions that accessed previously set options. Also while there's qobject_input_visitor_new_str, I couldn't find anything that would do the reverse. I wrote a quick and dirty function that does this, it appears to work, but unlike QemuOpts, QDict is unordered, and writing -audiodev id=foo,driver=alsa,rest... is IMHO more logical than writing them alphabetically. Or maybe it's just me who's too picky and it doesn't matter. I was thinking about creating an Audiodev (the qapi type) directly would be better, then somehow print it with reflection. While this is not a typical use of qapi, at least qmp_qom_list creates qapi objects directly, so I assume it's ok. This would save us from parsing essentially the same data twice, and the code to deal with an Audiodev is simpler that dealing with the generic QDicts and QObjects, but it would require some refactoring. The second problem: with my patched options visitor, nested structs were required in qapi, the options visitor unconditionally filled them. With qobject_input_visitor, I have to mark them optional, otherwise the user have to specify at least one option from the nested structs. Essentially, if you have something like this: { 'struct': 'Inner', 'data': [ '*foo': 'str' ] }, { 'struct': 'Outer', 'data': [ '*bar': 'Inner' ] } With the old parser you couldn't specify {}, while with qobject_input_visitor_new_str, you can's specify { 'bar': {} } without using JSON. I'm not sure which is the better, but with the old behavior at least you could mark nested structs as required and not worry about checking has_x every time. Now that I wrote all this down, I'm starting to think that qobject_input_visitor is good, and I should just properly check the fields (or give them default values). Rubber duck debugging, I think. Regards, Zoltan
Hi, > I was thinking about creating an Audiodev (the qapi type) directly would > be better, then somehow print it with reflection. While this is not a > typical use of qapi, at least qmp_qom_list creates qapi objects > directly, so I assume it's ok. Yes, it's perfectly fine. > The second problem: with my patched options visitor, nested structs were > required in qapi, the options visitor unconditionally filled them. With > qobject_input_visitor, I have to mark them optional, otherwise the user > have to specify at least one option from the nested structs. Can we just drop the nesting? Have a look at DisplayOptions (qapi struct used to store -display options). It's a union, has some common base fields, and the discriminator field (type in that case) decides which of the data branches is used. And on the command line I can do stuff like this: -display gtk,full-screen=on,zoom-to-fit=on ^^^^^^^^^^^ in struct DisplayGTK ^^^^^^^^^^^ common base field Audiodev should be able to do the same to support backend-specific options without nesting. cheers, Gerd
On 2019-01-10 08:25, Gerd Hoffmann wrote: > Hi, > >> I was thinking about creating an Audiodev (the qapi type) directly would >> be better, then somehow print it with reflection. While this is not a >> typical use of qapi, at least qmp_qom_list creates qapi objects >> directly, so I assume it's ok. > > Yes, it's perfectly fine. > >> The second problem: with my patched options visitor, nested structs were >> required in qapi, the options visitor unconditionally filled them. With >> qobject_input_visitor, I have to mark them optional, otherwise the user >> have to specify at least one option from the nested structs. > > Can we just drop the nesting? > > Have a look at DisplayOptions (qapi struct used to store -display > options). It's a union, has some common base fields, and the > discriminator field (type in that case) decides which of the data > branches is used. And on the command line I can do stuff like this: > > -display gtk,full-screen=on,zoom-to-fit=on > ^^^^^^^^^^^ in struct DisplayGTK > ^^^^^^^^^^^ common base field > > Audiodev should be able to do the same to support backend-specific > options without nesting. I think that union thing works, the problem is with input and output settings, because they're common. Currently you can write in.frequency and out.frequency, in and out are both AudiodevPerDirectionOptions. The alternative would be to flatten the whole structure and have in_frequency, out_frequency and everything else duplicated in Audiodev Regards, Zoltan
> > Can we just drop the nesting? > > > > Have a look at DisplayOptions (qapi struct used to store -display > > options). It's a union, has some common base fields, and the > > discriminator field (type in that case) decides which of the data > > branches is used. And on the command line I can do stuff like this: > > > > -display gtk,full-screen=on,zoom-to-fit=on > > ^^^^^^^^^^^ in struct DisplayGTK > > ^^^^^^^^^^^ common base field > > > > Audiodev should be able to do the same to support backend-specific > > options without nesting. > > I think that union thing works, the problem is with input and output > settings, because they're common. Currently you can write in.frequency > and out.frequency, in and out are both AudiodevPerDirectionOptions. Ah, right, I remember now. > The alternative would be to flatten the whole structure and have > in_frequency, out_frequency and everything else duplicated in Audiodev Which might be not that bad after all. The parser code is generated. The audio drivers (where the settings are looked up) have separate code paths for in and out anyway. So a per-direction struct probably wouldn't reduce the amout of code in the drivers. Driver-specific per-direction options are easier to deal with too I think (not sure we have them though). Property names should use dashes not underscores btw (i.e. in-frequency). cheers, Gerd
diff --git a/audio/Makefile.objs b/audio/Makefile.objs index db4fa7f18f..dca87f6347 100644 --- a/audio/Makefile.objs +++ b/audio/Makefile.objs @@ -1,4 +1,4 @@ -common-obj-y = audio.o noaudio.o wavaudio.o mixeng.o +common-obj-y = audio.o audio_legacy.o noaudio.o wavaudio.o mixeng.o common-obj-$(CONFIG_SPICE) += spiceaudio.o common-obj-$(CONFIG_AUDIO_COREAUDIO) += coreaudio.o common-obj-$(CONFIG_AUDIO_DSOUND) += dsoundaudio.o diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c index 5bd034267f..8302f3e882 100644 --- a/audio/alsaaudio.c +++ b/audio/alsaaudio.c @@ -1125,7 +1125,7 @@ static ALSAConf glob_conf = { .pcm_name_in = "default", }; -static void *alsa_audio_init (void) +static void *alsa_audio_init(Audiodev *dev) { ALSAConf *conf = g_malloc(sizeof(ALSAConf)); *conf = glob_conf; diff --git a/audio/audio.c b/audio/audio.c index 96cbd57c37..e7f25ea84b 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -25,7 +25,12 @@ #include "hw/hw.h" #include "audio.h" #include "monitor/monitor.h" +#include "qapi/opts-visitor.h" #include "qemu/timer.h" +#include "qemu/option.h" +#include "qemu/config-file.h" +#include "qapi/error.h" +#include "qapi/qapi-visit-audio.h" #include "sysemu/sysemu.h" #include "qemu/cutils.h" #include "sysemu/replay.h" @@ -46,11 +51,12 @@ The 1st one is the one used by default, that is the reason that we generate the list. */ -static const char *audio_prio_list[] = { +const char *audio_prio_list[] = { "spice", CONFIG_AUDIO_DRIVERS "none", "wav", + NULL }; static QLIST_HEAD(, audio_driver) audio_drivers; @@ -80,61 +86,6 @@ audio_driver *audio_driver_lookup(const char *name) return NULL; } -static void audio_module_load_all(void) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(audio_prio_list); i++) { - audio_driver_lookup(audio_prio_list[i]); - } -} - -struct fixed_settings { - int enabled; - int nb_voices; - int greedy; - struct audsettings settings; -}; - -static struct { - struct fixed_settings fixed_out; - struct fixed_settings fixed_in; - union { - int hertz; - int64_t ticks; - } period; - int try_poll_in; - int try_poll_out; -} conf = { - .fixed_out = { /* DAC fixed settings */ - .enabled = 1, - .nb_voices = 1, - .greedy = 1, - .settings = { - .freq = 44100, - .nchannels = 2, - .fmt = AUDIO_FORMAT_S16, - .endianness = AUDIO_HOST_ENDIANNESS, - } - }, - - .fixed_in = { /* ADC fixed settings */ - .enabled = 1, - .nb_voices = 1, - .greedy = 1, - .settings = { - .freq = 44100, - .nchannels = 2, - .fmt = AUDIO_FORMAT_S16, - .endianness = AUDIO_HOST_ENDIANNESS, - } - }, - - .period = { .hertz = 100 }, - .try_poll_in = 1, - .try_poll_out = 1, -}; - static AudioState glob_audio_state; const struct mixeng_volume nominal_volume = { @@ -151,9 +102,6 @@ const struct mixeng_volume nominal_volume = { #ifdef AUDIO_IS_FLAWLESS_AND_NO_CHECKS_ARE_REQURIED #error No its not #else -static void audio_print_options (const char *prefix, - struct audio_option *opt); - int audio_bug (const char *funcname, int cond) { if (cond) { @@ -161,16 +109,9 @@ int audio_bug (const char *funcname, int cond) AUD_log (NULL, "A bug was just triggered in %s\n", funcname); if (!shown) { - struct audio_driver *d; - shown = 1; AUD_log (NULL, "Save all your work and restart without audio\n"); - AUD_log (NULL, "Please send bug report to av1474@comtv.ru\n"); AUD_log (NULL, "I am sorry\n"); - d = glob_audio_state.drv; - if (d) { - audio_print_options (d->name, d->options); - } } AUD_log (NULL, "Context:\n"); @@ -232,31 +173,6 @@ void *audio_calloc (const char *funcname, int nmemb, size_t size) return g_malloc0 (len); } -static char *audio_alloc_prefix (const char *s) -{ - const char qemu_prefix[] = "QEMU_"; - size_t len, i; - char *r, *u; - - if (!s) { - return NULL; - } - - len = strlen (s); - r = g_malloc (len + sizeof (qemu_prefix)); - - u = r + sizeof (qemu_prefix) - 1; - - pstrcpy (r, len + sizeof (qemu_prefix), qemu_prefix); - pstrcat (r, len + sizeof (qemu_prefix), s); - - for (i = 0; i < len; ++i) { - u[i] = qemu_toupper(u[i]); - } - - return r; -} - static const char *audio_audfmt_to_string (AudioFormat fmt) { switch (fmt) { @@ -382,78 +298,6 @@ void AUD_log (const char *cap, const char *fmt, ...) va_end (ap); } -static void audio_print_options (const char *prefix, - struct audio_option *opt) -{ - char *uprefix; - - if (!prefix) { - dolog ("No prefix specified\n"); - return; - } - - if (!opt) { - dolog ("No options\n"); - return; - } - - uprefix = audio_alloc_prefix (prefix); - - for (; opt->name; opt++) { - const char *state = "default"; - printf (" %s_%s: ", uprefix, opt->name); - - if (opt->overriddenp && *opt->overriddenp) { - state = "current"; - } - - switch (opt->tag) { - case AUD_OPT_BOOL: - { - int *intp = opt->valp; - printf ("boolean, %s = %d\n", state, *intp ? 1 : 0); - } - break; - - case AUD_OPT_INT: - { - int *intp = opt->valp; - printf ("integer, %s = %d\n", state, *intp); - } - break; - - case AUD_OPT_FMT: - { - AudioFormat *fmtp = opt->valp; - printf ( - "format, %s = %s, (one of: U8 S8 U16 S16 U32 S32)\n", - state, - audio_audfmt_to_string (*fmtp) - ); - } - break; - - case AUD_OPT_STR: - { - const char **strp = opt->valp; - printf ("string, %s = %s\n", - state, - *strp ? *strp : "(not set)"); - } - break; - - default: - printf ("???\n"); - dolog ("Bad value tag for option %s_%s %d\n", - uprefix, opt->name, opt->tag); - break; - } - printf (" %s\n", opt->descr); - } - - g_free (uprefix); -} - static void audio_process_options (const char *prefix, struct audio_option *opt) { @@ -1161,11 +1005,11 @@ static void audio_reset_timer (AudioState *s) { if (audio_is_timer_needed ()) { timer_mod_anticipate_ns(s->ts, - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks); + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->period_ticks); if (!audio_timer_running) { audio_timer_running = true; audio_timer_last = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); - trace_audio_timer_start(conf.period.ticks / SCALE_MS); + trace_audio_timer_start(s->period_ticks / SCALE_MS); } } else { timer_del(s->ts); @@ -1179,16 +1023,17 @@ static void audio_reset_timer (AudioState *s) static void audio_timer (void *opaque) { int64_t now, diff; + AudioState *s = opaque; now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); diff = now - audio_timer_last; - if (diff > conf.period.ticks * 3 / 2) { + if (diff > s->period_ticks * 3 / 2) { trace_audio_timer_delayed(diff / SCALE_MS); } audio_timer_last = now; - audio_run ("timer"); - audio_reset_timer (opaque); + audio_run("timer"); + audio_reset_timer(s); } /* @@ -1248,7 +1093,7 @@ void AUD_set_active_out (SWVoiceOut *sw, int on) if (!hw->enabled) { hw->enabled = 1; if (s->vm_running) { - hw->pcm_ops->ctl_out (hw, VOICE_ENABLE, conf.try_poll_out); + hw->pcm_ops->ctl_out(hw, VOICE_ENABLE, true /* todo */); audio_reset_timer (s); } } @@ -1293,7 +1138,7 @@ void AUD_set_active_in (SWVoiceIn *sw, int on) if (!hw->enabled) { hw->enabled = 1; if (s->vm_running) { - hw->pcm_ops->ctl_in (hw, VOICE_ENABLE, conf.try_poll_in); + hw->pcm_ops->ctl_in(hw, VOICE_ENABLE, true /* todo */); audio_reset_timer (s); } } @@ -1614,169 +1459,13 @@ void audio_run (const char *msg) #endif } -static struct audio_option audio_options[] = { - /* DAC */ - { - .name = "DAC_FIXED_SETTINGS", - .tag = AUD_OPT_BOOL, - .valp = &conf.fixed_out.enabled, - .descr = "Use fixed settings for host DAC" - }, - { - .name = "DAC_FIXED_FREQ", - .tag = AUD_OPT_INT, - .valp = &conf.fixed_out.settings.freq, - .descr = "Frequency for fixed host DAC" - }, - { - .name = "DAC_FIXED_FMT", - .tag = AUD_OPT_FMT, - .valp = &conf.fixed_out.settings.fmt, - .descr = "Format for fixed host DAC" - }, - { - .name = "DAC_FIXED_CHANNELS", - .tag = AUD_OPT_INT, - .valp = &conf.fixed_out.settings.nchannels, - .descr = "Number of channels for fixed DAC (1 - mono, 2 - stereo)" - }, - { - .name = "DAC_VOICES", - .tag = AUD_OPT_INT, - .valp = &conf.fixed_out.nb_voices, - .descr = "Number of voices for DAC" - }, - { - .name = "DAC_TRY_POLL", - .tag = AUD_OPT_BOOL, - .valp = &conf.try_poll_out, - .descr = "Attempt using poll mode for DAC" - }, - /* ADC */ - { - .name = "ADC_FIXED_SETTINGS", - .tag = AUD_OPT_BOOL, - .valp = &conf.fixed_in.enabled, - .descr = "Use fixed settings for host ADC" - }, - { - .name = "ADC_FIXED_FREQ", - .tag = AUD_OPT_INT, - .valp = &conf.fixed_in.settings.freq, - .descr = "Frequency for fixed host ADC" - }, - { - .name = "ADC_FIXED_FMT", - .tag = AUD_OPT_FMT, - .valp = &conf.fixed_in.settings.fmt, - .descr = "Format for fixed host ADC" - }, - { - .name = "ADC_FIXED_CHANNELS", - .tag = AUD_OPT_INT, - .valp = &conf.fixed_in.settings.nchannels, - .descr = "Number of channels for fixed ADC (1 - mono, 2 - stereo)" - }, - { - .name = "ADC_VOICES", - .tag = AUD_OPT_INT, - .valp = &conf.fixed_in.nb_voices, - .descr = "Number of voices for ADC" - }, - { - .name = "ADC_TRY_POLL", - .tag = AUD_OPT_BOOL, - .valp = &conf.try_poll_in, - .descr = "Attempt using poll mode for ADC" - }, - /* Misc */ - { - .name = "TIMER_PERIOD", - .tag = AUD_OPT_INT, - .valp = &conf.period.hertz, - .descr = "Timer period in HZ (0 - use lowest possible)" - }, - { /* End of list */ } -}; - -static void audio_pp_nb_voices (const char *typ, int nb) -{ - switch (nb) { - case 0: - printf ("Does not support %s\n", typ); - break; - case 1: - printf ("One %s voice\n", typ); - break; - case INT_MAX: - printf ("Theoretically supports many %s voices\n", typ); - break; - default: - printf ("Theoretically supports up to %d %s voices\n", nb, typ); - break; - } - -} - -void AUD_help (void) -{ - struct audio_driver *d; - - /* make sure we print the help text for modular drivers too */ - audio_module_load_all(); - - audio_process_options ("AUDIO", audio_options); - QLIST_FOREACH(d, &audio_drivers, next) { - if (d->options) { - audio_process_options (d->name, d->options); - } - } - - printf ("Audio options:\n"); - audio_print_options ("AUDIO", audio_options); - printf ("\n"); - - printf ("Available drivers:\n"); - - QLIST_FOREACH(d, &audio_drivers, next) { - - printf ("Name: %s\n", d->name); - printf ("Description: %s\n", d->descr); - - audio_pp_nb_voices ("playback", d->max_voices_out); - audio_pp_nb_voices ("capture", d->max_voices_in); - - if (d->options) { - printf ("Options:\n"); - audio_print_options (d->name, d->options); - } - else { - printf ("No options\n"); - } - printf ("\n"); - } - - printf ( - "Options are settable through environment variables.\n" - "Example:\n" -#ifdef _WIN32 - " set QEMU_AUDIO_DRV=wav\n" - " set QEMU_WAV_PATH=c:\\tune.wav\n" -#else - " export QEMU_AUDIO_DRV=wav\n" - " export QEMU_WAV_PATH=$HOME/tune.wav\n" - "(for csh replace export with setenv in the above)\n" -#endif - " qemu ...\n\n" - ); -} - -static int audio_driver_init (AudioState *s, struct audio_driver *drv) +static int audio_driver_init(AudioState *s, struct audio_driver *drv, + Audiodev *dev) { if (drv->options) { audio_process_options (drv->name, drv->options); } - s->drv_opaque = drv->init (); + s->drv_opaque = drv->init(dev); if (s->drv_opaque) { audio_init_nb_voices_out (drv); @@ -1800,11 +1489,11 @@ static void audio_vm_change_state_handler (void *opaque, int running, s->vm_running = running; while ((hwo = audio_pcm_hw_find_any_enabled_out (hwo))) { - hwo->pcm_ops->ctl_out (hwo, op, conf.try_poll_out); + hwo->pcm_ops->ctl_out(hwo, op, true /* todo */); } while ((hwi = audio_pcm_hw_find_any_enabled_in (hwi))) { - hwi->pcm_ops->ctl_in (hwi, op, conf.try_poll_in); + hwi->pcm_ops->ctl_in(hwi, op, true /* todo */); } audio_reset_timer (s); } @@ -1854,6 +1543,11 @@ void audio_cleanup(void) s->drv->fini (s->drv_opaque); s->drv = NULL; } + + if (s->dev) { + qapi_free_Audiodev(s->dev); + s->dev = NULL; + } } static const VMStateDescription vmstate_audio = { @@ -1865,19 +1559,40 @@ static const VMStateDescription vmstate_audio = { } }; -static void audio_init (void) +static Audiodev *parse_option(QemuOpts *opts, Error **errp); +static int audio_init(Audiodev *dev) { size_t i; int done = 0; - const char *drvname; + const char *drvname = NULL; VMChangeStateEntry *e; AudioState *s = &glob_audio_state; struct audio_driver *driver; + /* silence gcc warning about uninitialized variable */ + QemuOptsList *list = NULL; if (s->drv) { - return; + if (dev) { + dolog("Cannot create more than one audio backend, sorry\n"); + qapi_free_Audiodev(dev); + } + return -1; } + if (dev) { + /* -audiodev option */ + drvname = AudiodevDriver_str(dev->driver); + } else { + /* legacy implicit initialization */ + audio_handle_legacy_opts(); + list = qemu_find_opts("audiodev"); + dev = parse_option(QTAILQ_FIRST(&list->head), &error_abort); + if (!dev) { + exit(1); + } + } + s->dev = dev; + QLIST_INIT (&s->hw_head_out); QLIST_INIT (&s->hw_head_in); QLIST_INIT (&s->cap_head); @@ -1885,10 +1600,8 @@ static void audio_init (void) s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s); - audio_process_options ("AUDIO", audio_options); - - s->nb_hw_voices_out = conf.fixed_out.nb_voices; - s->nb_hw_voices_in = conf.fixed_in.nb_voices; + s->nb_hw_voices_out = dev->out->voices; + s->nb_hw_voices_in = dev->in->voices; if (s->nb_hw_voices_out <= 0) { dolog ("Bogus number of playback voices %d, setting to 1\n", @@ -1902,46 +1615,46 @@ static void audio_init (void) s->nb_hw_voices_in = 0; } - { - int def; - drvname = audio_get_conf_str ("QEMU_AUDIO_DRV", NULL, &def); - } - if (drvname) { driver = audio_driver_lookup(drvname); if (driver) { - done = !audio_driver_init(s, driver); + done = !audio_driver_init(s, driver, dev); } else { dolog ("Unknown audio driver `%s'\n", drvname); - dolog ("Run with -audio-help to list available drivers\n"); } - } - - if (!done) { - for (i = 0; !done && i < ARRAY_SIZE(audio_prio_list); i++) { + } else { + for (i = 0; !done && audio_prio_list[i]; i++) { + QemuOpts *opts = qemu_opts_find(list, audio_prio_list[i]); driver = audio_driver_lookup(audio_prio_list[i]); - if (driver && driver->can_be_default) { - done = !audio_driver_init(s, driver); + + if (driver && opts) { + qapi_free_Audiodev(dev); + dev = parse_option(opts, &error_abort); + if (!dev) { + exit(1); + } + s->dev = dev; + done = !audio_driver_init(s, driver, dev); } } } if (!done) { driver = audio_driver_lookup("none"); - done = !audio_driver_init(s, driver); + done = !audio_driver_init(s, driver, dev); assert(done); dolog("warning: Using timer based audio emulation\n"); } - if (conf.period.hertz <= 0) { - if (conf.period.hertz < 0) { - dolog ("warning: Timer period is negative - %d " - "treating as zero\n", - conf.period.hertz); + if (dev->timer_period <= 0) { + if (dev->timer_period < 0) { + dolog ("warning: Timer period is negative - %" PRId64 + " treating as zero\n", + dev->timer_period); } - conf.period.ticks = 1; + s->period_ticks = 1; } else { - conf.period.ticks = NANOSECONDS_PER_SECOND / conf.period.hertz; + s->period_ticks = NANOSECONDS_PER_SECOND / dev->timer_period; } e = qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s); @@ -1952,11 +1665,12 @@ static void audio_init (void) QLIST_INIT (&s->card_head); vmstate_register (NULL, 0, &vmstate_audio, s); + return 0; } void AUD_register_card (const char *name, QEMUSoundCard *card) { - audio_init (); + audio_init(NULL); card->name = g_strdup (name); memset (&card->entries, 0, sizeof (card->entries)); QLIST_INSERT_HEAD (&glob_audio_state.card_head, card, entries); @@ -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); + } +} + +audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo) +{ + return (audsettings) { + .freq = pdo->frequency, + .nchannels = pdo->channels, + .fmt = pdo->format, + .endianness = AUDIO_HOST_ENDIANNESS, + }; +} + +int audioformat_bytes_per_sample(AudioFormat fmt) +{ + switch (fmt) { + case AUDIO_FORMAT_U8: + case AUDIO_FORMAT_S8: + return 1; + + case AUDIO_FORMAT_U16: + case AUDIO_FORMAT_S16: + return 2; + + case AUDIO_FORMAT_U32: + case AUDIO_FORMAT_S32: + return 4; + + case AUDIO_FORMAT__MAX: + ; + } + abort(); +} + + +/* frames = freq * usec / 1e6 */ +int audio_buffer_frames(AudiodevPerDirectionOptions *pdo, + audsettings *as, int def_usecs) +{ + uint64_t usecs = pdo->has_buffer_len ? pdo->buffer_len : def_usecs; + return (as->freq * usecs + 500000) / 1000000; +} + +/* samples = channels * frames = channels * freq * usec / 1e6 */ +int audio_buffer_samples(AudiodevPerDirectionOptions *pdo, + audsettings *as, int def_usecs) +{ + return as->nchannels * audio_buffer_frames(pdo, as, def_usecs); +} + +/* + * bytes = bytes_per_sample * samples = + * bytes_per_sample * channels * freq * usec / 1e6 + */ +int audio_buffer_bytes(AudiodevPerDirectionOptions *pdo, + audsettings *as, int def_usecs) +{ + return audio_buffer_samples(pdo, as, def_usecs) * + audioformat_bytes_per_sample(as->fmt); +} diff --git a/audio/audio.h b/audio/audio.h index 02f29a3b3e..7df1b8b249 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, @@ -78,10 +87,11 @@ typedef struct QEMUAudioTimeStamp { uint64_t old_ts; } QEMUAudioTimeStamp; +extern QemuOptsList qemu_audiodev_opts; + 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 +173,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_set_options(void); +void audio_handle_legacy_opts(void); +void audio_legacy_help(void); + #endif /* QEMU_AUDIO_H */ diff --git a/audio/audio_int.h b/audio/audio_int.h index 244b454012..24b8793496 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 { struct AudioState { struct audio_driver *drv; + Audiodev *dev; void *drv_opaque; QEMUTimer *ts; @@ -203,10 +204,13 @@ struct AudioState { int nb_hw_voices_out; int nb_hw_voices_in; int vm_running; + int64_t period_ticks; }; 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); diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c new file mode 100644 index 0000000000..2242f06a5e --- /dev/null +++ b/audio/audio_legacy.c @@ -0,0 +1,211 @@ +#include "qemu/osdep.h" +#include "audio.h" +#include "audio_int.h" +#include "qemu-common.h" +#include "qemu/config-file.h" +#include "qemu/cutils.h" +#include "qemu/option.h" +#include "qapi/error.h" + +#define AUDIO_CAP "audio-legacy" +#include "audio_int.h" + +typedef enum EnvTransform { + ENV_TRANSFORM_NONE, + ENV_TRANSFORM_BOOL, + ENV_TRANSFORM_FMT, + ENV_TRANSFORM_FRAMES_TO_USECS_IN, + ENV_TRANSFORM_FRAMES_TO_USECS_OUT, + ENV_TRANSFORM_SAMPLES_TO_USECS_IN, + ENV_TRANSFORM_SAMPLES_TO_USECS_OUT, + ENV_TRANSFORM_BYTES_TO_USECS_IN, + ENV_TRANSFORM_BYTES_TO_USECS_OUT, + ENV_TRANSFORM_MILLIS_TO_USECS, + ENV_TRANSFORM_HZ_TO_USECS, +} EnvTransform; + +typedef struct SimpleEnvMap { + const char *name; + const char *option; + EnvTransform transform; +} SimpleEnvMap; + +SimpleEnvMap global_map[] = { + /* DAC/out settings */ + { "QEMU_AUDIO_DAC_FIXED_SETTINGS", "out.fixed-settings", + ENV_TRANSFORM_BOOL }, + { "QEMU_AUDIO_DAC_FIXED_FREQ", "out.frequency" }, + { "QEMU_AUDIO_DAC_FIXED_FMT", "out.format", ENV_TRANSFORM_FMT }, + { "QEMU_AUDIO_DAC_FIXED_CHANNELS", "out.channels" }, + { "QEMU_AUDIO_DAC_VOICES", "out.voices" }, + + /* ADC/in settings */ + { "QEMU_AUDIO_ADC_FIXED_SETTINGS", "in.fixed-settings", + ENV_TRANSFORM_BOOL }, + { "QEMU_AUDIO_ADC_FIXED_FREQ", "in.frequency" }, + { "QEMU_AUDIO_ADC_FIXED_FMT", "in.format", ENV_TRANSFORM_FMT }, + { "QEMU_AUDIO_ADC_FIXED_CHANNELS", "in.channels" }, + { "QEMU_AUDIO_ADC_VOICES", "in.voices" }, + + /* general */ + { "QEMU_AUDIO_TIMER_PERIOD", "timer-period", ENV_TRANSFORM_HZ_TO_USECS }, + { /* End of list */ } +}; + +static unsigned long long toull(const char *str) +{ + unsigned long long ret; + if (parse_uint_full(str, &ret, 10)) { + dolog("Invalid integer value `%s'\n", str); + exit(1); + } + return ret; +} + +/* non reentrant typesafe or anything, but enough in this small c file */ +static const char *tostr(unsigned long long val) +{ + /* max length in decimal possible for an unsigned long long number */ + #define LEN ((CHAR_BIT * sizeof(unsigned long long) - 1) / 3 + 2) + static char ret[LEN]; + snprintf(ret, LEN, "%llu", val); + return ret; +} + +static uint64_t frames_to_usecs(QemuOpts *opts, uint64_t frames, bool in) +{ + const char *opt = in ? "in.frequency" : "out.frequency"; + uint64_t freq = qemu_opt_get_number(opts, opt, 44100); + return (frames * 1000000 + freq / 2) / freq; +} + +static uint64_t samples_to_usecs(QemuOpts *opts, uint64_t samples, bool in) +{ + const char *opt = in ? "in.channels" : "out.channels"; + uint64_t channels = qemu_opt_get_number(opts, opt, 2); + return frames_to_usecs(opts, samples / channels, in); +} + +static uint64_t bytes_to_usecs(QemuOpts *opts, uint64_t bytes, bool in) +{ + const char *opt = in ? "in.format" : "out.format"; + const char *val = qemu_opt_get(opts, opt); + uint64_t bytes_per_sample = (val ? toull(val) : 16) / 8; + return samples_to_usecs(opts, bytes * bytes_per_sample, in); +} + +static const char *transform_val(QemuOpts *opts, const char *val, + EnvTransform transform) +{ + switch (transform) { + case ENV_TRANSFORM_NONE: + return val; + + case ENV_TRANSFORM_BOOL: + return toull(val) ? "on" : "off"; + + case ENV_TRANSFORM_FMT: + if (strcasecmp(val, "u8") == 0) { + return "u8"; + } else if (strcasecmp(val, "u16") == 0) { + return "u16"; + } else if (strcasecmp(val, "u32") == 0) { + return "u32"; + } else if (strcasecmp(val, "s8") == 0) { + return "s8"; + } else if (strcasecmp(val, "s16") == 0) { + return "s16"; + } else if (strcasecmp(val, "s32") == 0) { + return "s32"; + } else { + dolog("Invalid audio format `%s'\n", val); + exit(1); + } + + case ENV_TRANSFORM_FRAMES_TO_USECS_IN: + return tostr(frames_to_usecs(opts, toull(val), true)); + case ENV_TRANSFORM_FRAMES_TO_USECS_OUT: + return tostr(frames_to_usecs(opts, toull(val), false)); + + case ENV_TRANSFORM_SAMPLES_TO_USECS_IN: + return tostr(samples_to_usecs(opts, toull(val), true)); + case ENV_TRANSFORM_SAMPLES_TO_USECS_OUT: + return tostr(samples_to_usecs(opts, toull(val), false)); + + case ENV_TRANSFORM_BYTES_TO_USECS_IN: + return tostr(bytes_to_usecs(opts, toull(val), true)); + case ENV_TRANSFORM_BYTES_TO_USECS_OUT: + return tostr(bytes_to_usecs(opts, toull(val), false)); + + case ENV_TRANSFORM_MILLIS_TO_USECS: + return tostr(toull(val) * 1000); + + case ENV_TRANSFORM_HZ_TO_USECS: + return tostr(1000000 / toull(val)); + } + + abort(); /* it's unreachable, gcc */ +} + +static void handle_env_opts(QemuOpts *opts, SimpleEnvMap *map) +{ + while (map->name) { + const char *val = getenv(map->name); + + if (val) { + qemu_opt_set(opts, map->option, + transform_val(opts, val, map->transform), + &error_abort); + } + + ++map; + } +} + +static void legacy_opt(const char *drv) +{ + QemuOpts *opts; + opts = qemu_opts_create(qemu_find_opts("audiodev"), drv, true, + &error_abort); + qemu_opt_set(opts, "driver", drv, &error_abort); + + handle_env_opts(opts, global_map); +} + +void audio_handle_legacy_opts(void) +{ + const char *drvname = getenv("QEMU_AUDIO_DRV"); + + if (drvname) { + audio_driver *driver = audio_driver_lookup(drvname); + if (!driver) { + dolog("Unknown audio driver `%s'\n", drvname); + } + legacy_opt(drvname); + } else { + for (int i = 0; audio_prio_list[i]; i++) { + audio_driver *driver = audio_driver_lookup(audio_prio_list[i]); + if (driver && driver->can_be_default) { + legacy_opt(driver->name); + } + } + } +} + +static int legacy_help_each(void *opaque, QemuOpts *opts, Error **errp) +{ + printf("-audiodev "); + qemu_opts_print(opts, ","); + printf("\n"); + return 0; +} + +void audio_legacy_help(void) +{ + printf("Environment variable based configuration deprecated.\n"); + printf("Please use the new -audiodev option.\n"); + + audio_handle_legacy_opts(); + printf("\nEquivalent -audiodev to your current environment variables:\n"); + qemu_opts_foreach(qemu_find_opts("audiodev"), legacy_help_each, NULL, NULL); +} 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 *hw; + AudioState *s = &glob_audio_state; + AudiodevPerDirectionOptions *pdo = s->dev->TYPE; - if (glue (conf.fixed_, TYPE).enabled && glue (conf.fixed_, TYPE).greedy) { + if (pdo->fixed_settings) { hw = glue (audio_pcm_hw_add_new_, TYPE) (as); if (hw) { return hw; @@ -331,9 +333,11 @@ 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 (glue (conf.fixed_, TYPE).enabled) { - hw_as = glue (conf.fixed_, TYPE).settings; + if (pdo->fixed_settings) { + hw_as = audiodev_to_audsettings(pdo); } else { hw_as = *as; @@ -398,6 +402,7 @@ SW *glue (AUD_open_, TYPE) ( ) { AudioState *s = &glob_audio_state; + AudiodevPerDirectionOptions *pdo = s->dev->TYPE; if (audio_bug(__func__, !card || !name || !callback_fn || !as)) { dolog ("card=%p name=%p callback_fn=%p as=%p\n", @@ -422,7 +427,7 @@ SW *glue (AUD_open_, TYPE) ( return sw; } - if (!glue (conf.fixed_, TYPE).enabled && sw) { + if (!pdo->fixed_settings && sw) { glue (AUD_close_, TYPE) (card, sw); sw = NULL; } diff --git a/audio/coreaudio.c b/audio/coreaudio.c index 638c60b300..7d4225dbee 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -685,7 +685,7 @@ static CoreaudioConf glob_conf = { .nbuffers = 4, }; -static void *coreaudio_audio_init (void) +static void *coreaudio_audio_init(Audiodev *dev) { CoreaudioConf *conf = g_malloc(sizeof(CoreaudioConf)); *conf = glob_conf; diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c index 3ed73a30d1..02fe777cba 100644 --- a/audio/dsoundaudio.c +++ b/audio/dsoundaudio.c @@ -783,7 +783,7 @@ static void dsound_audio_fini (void *opaque) g_free(s); } -static void *dsound_audio_init (void) +static void *dsound_audio_init(Audiodev *dev) { int err; HRESULT hr; diff --git a/audio/noaudio.c b/audio/noaudio.c index 1bfebeca7d..2cc274c5e5 100644 --- a/audio/noaudio.c +++ b/audio/noaudio.c @@ -136,7 +136,7 @@ static int no_ctl_in (HWVoiceIn *hw, int cmd, ...) return 0; } -static void *no_audio_init (void) +static void *no_audio_init (Audiodev *dev) { return &no_audio_init; } diff --git a/audio/ossaudio.c b/audio/ossaudio.c index 355e8fbda5..e0cadbef29 100644 --- a/audio/ossaudio.c +++ b/audio/ossaudio.c @@ -842,7 +842,7 @@ static OSSConf glob_conf = { .policy = 5 }; -static void *oss_audio_init (void) +static void *oss_audio_init(Audiodev *dev) { OSSConf *conf = g_malloc(sizeof(OSSConf)); *conf = glob_conf; diff --git a/audio/paaudio.c b/audio/paaudio.c index f1f9a741ac..0981f010c9 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -812,7 +812,7 @@ static PAConf glob_conf = { .samples = 4096, }; -static void *qpa_audio_init (void) +static void *qpa_audio_init(Audiodev *dev) { paaudio *g = g_malloc(sizeof(paaudio)); g->conf = glob_conf; diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c index aa42ea26bf..097841fde1 100644 --- a/audio/sdlaudio.c +++ b/audio/sdlaudio.c @@ -436,7 +436,7 @@ static int sdl_ctl_out (HWVoiceOut *hw, int cmd, ...) return 0; } -static void *sdl_audio_init (void) +static void *sdl_audio_init(Audiodev *dev) { SDLAudioState *s = &glob_sdl; if (s->driver_created) { diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c index 3aeb0cb357..affc3df17f 100644 --- a/audio/spiceaudio.c +++ b/audio/spiceaudio.c @@ -77,7 +77,7 @@ static const SpiceRecordInterface record_sif = { .base.minor_version = SPICE_INTERFACE_RECORD_MINOR, }; -static void *spice_audio_init (void) +static void *spice_audio_init(Audiodev *dev) { if (!using_spice) { return NULL; diff --git a/audio/wavaudio.c b/audio/wavaudio.c index 35a614785e..9eff3555b3 100644 --- a/audio/wavaudio.c +++ b/audio/wavaudio.c @@ -232,7 +232,7 @@ static WAVConf glob_conf = { .wav_path = "qemu.wav" }; -static void *wav_audio_init (void) +static void *wav_audio_init(Audiodev *dev) { WAVConf *conf = g_malloc(sizeof(WAVConf)); *conf = glob_conf; 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);
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 <DirtY.iCE.hu@gmail.com> --- audio/Makefile.objs | 2 +- audio/alsaaudio.c | 2 +- audio/audio.c | 589 ++++++++++++++++------------------------- audio/audio.h | 20 +- audio/audio_int.h | 6 +- audio/audio_legacy.c | 211 +++++++++++++++ audio/audio_template.h | 13 +- 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 | 11 +- 16 files changed, 491 insertions(+), 379 deletions(-) create mode 100644 audio/audio_legacy.c