* Markus Armbruster (arm...@redhat.com) wrote: > "Kővágó, Zoltán" <dirty.ice...@gmail.com> writes: > > > Signed-off-by: Kővágó, Zoltán <dirty.ice...@gmail.com> > > --- > > ui/vnc.h | 2 ++ > > monitor/misc.c | 12 +++++++++++- > > ui/vnc.c | 15 ++++++++++++++- > > hmp-commands.hx | 13 ++++++++----- > > qemu-options.hx | 6 ++++++ > > 5 files changed, 41 insertions(+), 7 deletions(-) > > > > 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/monitor/misc.c b/monitor/misc.c > > index e393333a0e..f97810d370 100644 > > --- a/monitor/misc.c > > +++ b/monitor/misc.c > > @@ -1148,7 +1148,17 @@ static void hmp_wavcapture(Monitor *mon, const QDict > > *qdict) > > int bits = qdict_get_try_int(qdict, "bits", -1); > > int has_channels = qdict_haskey(qdict, "nchannels"); > > int nchannels = qdict_get_try_int(qdict, "nchannels", -1); > > + const char *audiodev = qdict_get_try_str(qdict, "audiodev"); > > CaptureState *s; > > + AudioState *as = NULL; > > + > > + if (audiodev) { > > + as = audio_state_by_name(audiodev); > > + if (!as) { > > + monitor_printf(mon, "Invalid audiodev specified\n"); > > + return; > > + } > > + } > > Note for later: if "audiodev" is specified, it must name an existing > AudioState. > > > > > s = g_malloc0 (sizeof (*s)); > > > > @@ -1156,7 +1166,7 @@ static void hmp_wavcapture(Monitor *mon, const QDict > > *qdict) > > bits = has_bits ? bits : 16; > > nchannels = has_channels ? nchannels : 2; > > > > - if (wav_start_capture(NULL, s, path, freq, bits, nchannels)) { > > + if (wav_start_capture(as, s, path, freq, bits, nchannels)) { > > monitor_printf(mon, "Failed to add wave capture\n"); > > g_free (s); > > return; > > Note for later: this is the only other failure mode. > > > diff --git a/ui/vnc.c b/ui/vnc.c > > index 140f364dda..24f9be5b5d 100644 > > --- a/ui/vnc.c > > +++ b/ui/vnc.c > > @@ -1222,7 +1222,7 @@ static void audio_add(VncState *vs) > > ops.destroy = audio_capture_destroy; > > ops.capture = audio_capture; > > > > - vs->audio_cap = AUD_add_capture(NULL, &vs->as, &ops, vs); > > + vs->audio_cap = AUD_add_capture(vs->vd->audio_state, &vs->as, &ops, > > vs); > > if (!vs->audio_cap) { > > error_report("Failed to add audio capture"); > > } > > @@ -3369,6 +3369,9 @@ static QemuOptsList qemu_vnc_opts = { > > },{ > > .name = "non-adaptive", > > .type = QEMU_OPT_BOOL, > > + },{ > > + .name = "audiodev", > > + .type = QEMU_OPT_STRING, > > }, > > { /* end of list */ } > > }, > > @@ -3806,6 +3809,7 @@ void vnc_display_open(const char *id, Error **errp) > > const char *saslauthz; > > int lock_key_sync = 1; > > int key_delay_ms; > > + const char *audiodev; > > > > if (!vd) { > > error_setg(errp, "VNC display not active"); > > @@ -3991,6 +3995,15 @@ void vnc_display_open(const char *id, Error **errp) > > } > > vd->ledstate = 0; > > > > + audiodev = qemu_opt_get(opts, "audiodev"); > > + if (audiodev) { > > + vd->audio_state = audio_state_by_name(audiodev); > > + if (!vd->audio_state) { > > + error_setg(errp, "Audiodev '%s' not found", audiodev); > > + goto fail; > > + } > > + } > > Note for later: if "audiodev" is specified, it must name an existing > AudioState. > > I like this error message better than the one in hmp_wavcapture(). Use > it there, too? > > Move it into audio_state_by_name() by giving it an Error **errp > parameter? Matter of taste, up to you. > > > + > > device_id = qemu_opt_get(opts, "display"); > > if (device_id) { > > int head = qemu_opt_get_number(opts, "head", 0); > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > index bfa5681dd2..fa7f009268 100644 > > --- a/hmp-commands.hx > > +++ b/hmp-commands.hx > > @@ -819,16 +819,19 @@ 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}. When not using an -audiodev argument on command line, > > +@var{audiodev} must be omitted, otherwise is must specify a valid > > +audiodev. > > I can see the code for "must specify a valid audiodev" in > hmp_wavcapture(). Where is "must be omitted" checked?
Isn't that just that there wont be any named devices so you wont be able to specify a valid one? Dave > Preexisting: the list "sample rate @var{frequency} bits per sample > @var{bits} and number of channels @var{channels}" lacks a comma after > @var{frequency}, please fix that. I'd put one after @var{bits} as well, > but that's a matter of taste[*] > > The sentence is of the form "if not COND then A else B". The > less-negated form "if COND then B else A" is commonly easier to read. > > Documentation says "from @var{audiodev}". But when "not using an > -audiodev argument on command line, +@var{audiodev} must be omitted". > Where does it sample from then? I figure from some default audio > device. Where is that default audio device explained? I skimmed the > -audiodev documentation in qemu-options.hx, but couldn't see it there. > > Suggest to say "an -audiodev command line option" instead of "an > -audiodev argument on command line". > > Double-checking: > > * -audiodev is the only way to create an audio backend. > > * If the user creates no audio backend, QEMU supplies a default audio > backend. > > Correct? > > Other kinds of backends can also be created at run-time with the > monitor. I'm not asking you provide that for audio. I'm just wondering > whether it could conceivably be useful. > > If yes, you might want to avoid the narrow "if using -audiodev", and > instead say "if the default audio device is in use". > > > > > Defaults: > > @itemize @minus > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 9621e934c0..a308e5f5aa 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -1978,6 +1978,12 @@ 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. When not using an -audiodev argument, this option must > > +be omitted, otherwise is must be present and specify a valid audiodev. > > + > > @end table > > ETEXI > > Same as for wavcapture, basically. > > > [*] https://en.wikipedia.org/wiki/Serial_comma -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK