| Message ID | 82a755097c6aaa16363348599110f9af3bb07583.1547681517.git.DirtY.iCE.hu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Audio 5.1 patches | expand |
On Thu, Jan 17, 2019 at 12:37:20AM +0100, Kővágó, Zoltán wrote: > Add an option to change the channel map used by pulseaudio. If not > specified, falls back to an OSS compatible channel map. > > Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> > --- > qapi/audio.json | 5 ++++- > audio/paaudio.c | 18 ++++++++++++++---- > qemu-options.hx | 9 +++++++++ > 3 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/qapi/audio.json b/qapi/audio.json > index 7bcea6240f..86078039dc 100644 > --- a/qapi/audio.json > +++ b/qapi/audio.json > @@ -107,11 +107,14 @@ > # > # @name: name of the sink/source to use > # > +# @channel-map: channel map to use (default: OSS compatible map) > +# > # Since: 4.0 > ## > { 'struct': 'AudiodevPaPerDirectionOptions', > 'data': { > - '*name': 'str' } } > + '*name': 'str', > + '*channel-map': 'str' } } Ah, I see. Thats why patch #1 creates a AudiodevPaPerDirectionOptions struct with just one field ... cheers, Gerd
On 2019-01-17 11:03, Gerd Hoffmann wrote: > On Thu, Jan 17, 2019 at 12:37:20AM +0100, Kővágó, Zoltán wrote: >> Add an option to change the channel map used by pulseaudio. If not >> specified, falls back to an OSS compatible channel map. >> >> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> >> --- >> qapi/audio.json | 5 ++++- >> audio/paaudio.c | 18 ++++++++++++++---- >> qemu-options.hx | 9 +++++++++ >> 3 files changed, 27 insertions(+), 5 deletions(-) >> >> diff --git a/qapi/audio.json b/qapi/audio.json >> index 7bcea6240f..86078039dc 100644 >> --- a/qapi/audio.json >> +++ b/qapi/audio.json >> @@ -107,11 +107,14 @@ >> # >> # @name: name of the sink/source to use >> # >> +# @channel-map: channel map to use (default: OSS compatible map) >> +# >> # Since: 4.0 >> ## >> { 'struct': 'AudiodevPaPerDirectionOptions', >> 'data': { >> - '*name': 'str' } } >> + '*name': 'str', >> + '*channel-map': 'str' } } > > Ah, I see. Thats why patch #1 creates a AudiodevPaPerDirectionOptions > struct with just one field ... Yes, I was bitten by it too during a refactor, probably I should write a few words about it in the commit message. However, this is the exact reason I'd recommend nested structs instead of randomly flattening it when we can. This way, if we later have to add an extra option, we don't end up in a problematic situation, since we can't easily change things like 'dev-in' to a structure without breaking backward compatibility. Regards, Zoltan
On 1/23/19 2:13 PM, Zoltán Kővágó wrote: >>> { 'struct': 'AudiodevPaPerDirectionOptions', >>> 'data': { >>> - '*name': 'str' } } >>> + '*name': 'str', >>> + '*channel-map': 'str' } } >> >> Ah, I see. Thats why patch #1 creates a AudiodevPaPerDirectionOptions >> struct with just one field ... > > Yes, I was bitten by it too during a refactor, probably I should write a > few words about it in the commit message. Yes, a good commit message tries to anticipate the questions the reviewer will ask. The "what" is easy to see (read the patch), but the "why" is the one that determines whether the patch is worth including. > > However, this is the exact reason I'd recommend nested structs instead > of randomly flattening it when we can. This way, if we later have to > add an extra option, we don't end up in a problematic situation, since > we can't easily change things like 'dev-in' to a structure without > breaking backward compatibility. We can support QAPI Alternates, accepting either a string or a struct for a given member name like 'dev-in'. Admittedly, it's a bit harder to write C code interfacing with a QAPI alternate type, but at least it is possible and allows for back-compatibility. Then again, getting the struct right in the first place is even nicer than having to make back-compat tweaks.
diff --git a/qapi/audio.json b/qapi/audio.json index 7bcea6240f..86078039dc 100644 --- a/qapi/audio.json +++ b/qapi/audio.json @@ -107,11 +107,14 @@ # # @name: name of the sink/source to use # +# @channel-map: channel map to use (default: OSS compatible map) +# # Since: 4.0 ## { 'struct': 'AudiodevPaPerDirectionOptions', 'data': { - '*name': 'str' } } + '*name': 'str', + '*channel-map': 'str' } } ## # @AudiodevPaOptions: diff --git a/audio/paaudio.c b/audio/paaudio.c index 0d67c03b4c..b0e311584a 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -337,17 +337,27 @@ static pa_stream *qpa_simple_new ( pa_stream_direction_t dir, const char *dev, const pa_sample_spec *ss, - const pa_channel_map *map, + const char *map, const pa_buffer_attr *attr, int *rerror) { int r; pa_stream *stream; pa_stream_flags_t flags; + pa_channel_map pa_map; pa_threaded_mainloop_lock(c->mainloop); - stream = pa_stream_new(c->context, name, ss, map); + if (map && !pa_channel_map_parse(&pa_map, map)) { + dolog("Invalid channel map specified: '%s'\n", map); + map = NULL; + } + if (!map) { + pa_channel_map_init_extend(&pa_map, ss->channels, + PA_CHANNEL_MAP_OSS); + } + + stream = pa_stream_new(c->context, name, ss, &pa_map); if (!stream) { goto fail; } @@ -424,7 +434,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings *as, PA_STREAM_PLAYBACK, ppdo->has_name ? ppdo->name : NULL, &ss, - NULL, /* channel map */ + ppdo->has_channel_map ? ppdo->channel_map : NULL, &ba, /* buffering attributes */ &error ); @@ -472,7 +482,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque) PA_STREAM_RECORD, ppdo->has_name ? ppdo->name : NULL, &ss, - NULL, /* channel map */ + ppdo->has_channel_map ? ppdo->channel_map : NULL, NULL, /* buffering attributes */ &error ); diff --git a/qemu-options.hx b/qemu-options.hx index 53fc5ef453..c4835f77c3 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -468,6 +468,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev, "-audiodev pa,id=id[,prop[=value][,...]]\n" " server= PulseAudio server address\n" " sink|source.name= sink/source device name\n" + " sink|source.channel-map= channel map to use\n" #endif #ifdef CONFIG_SDL "-audiodev sdl,id=id[,prop[=value][,...]]\n" @@ -621,6 +622,14 @@ Sets the PulseAudio @var{server} to connect to. @item sink|source.name=@var{sink} Use the specified sink/source for playback/recording. +@item sink|source.channel-map=@var{map} +Use the specified channel map. The default is an OSS compatible +channel map. Do not forget to escape commas inside the map: + +@example +-audiodev pa,id=example,sink.channel-map=front-left,,front-right +@end example + @end table @item -audiodev sdl,id=@var{id}[,@var{prop}[=@var{value}][,...]]
Add an option to change the channel map used by pulseaudio. If not specified, falls back to an OSS compatible channel map. Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> --- qapi/audio.json | 5 ++++- audio/paaudio.c | 18 ++++++++++++++---- qemu-options.hx | 9 +++++++++ 3 files changed, 27 insertions(+), 5 deletions(-)