Re: [PATCH v5 0/7] New API for Bluetooth A2DP codecs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 25 Jan 2019, at 3:19 PM, Tanu Kaskinen wrote:
> On Sat, 2019-01-19 at 18:11 +0100, Pali Rohár wrote:
> > This is 5th version of my patch series for modular A2DP codec API and
> > aptX support. The only change for v5 is support for switching codecs.
> > 
> > This patch series provides new modular API for Bluetooth A2DP codecs,
> > clean up module-bluez5-device and bluez5-util to be codec independent
> > and convert SBC codec into this new API for A2DP codecs. Also it adds
> > support for aptX, aptX HD and FastStream A2DP codecs.
> > 
> > New codec API is designed in way, that for adding new codec is not
> > needed to touch bluez5-util nor module-bluez5-device files. Whole
> > codec registration is done in a2dp-codec-util.c file, without need
> > to update any header file.
> > 
> > Some A2DP codecs are bidirectional and support backchannel for
> > microphone voice. This new A2DP codec API fully supports this feature
> > and module-bluez5-device was extended to support microphone voice source
> > for codecs which declares such support. FastStream is such codec.
> > 
> > For every A2DP codec is created card profile. When using bluez patches
> > from https://marc.info/?l=linux-bluetooth&m=154696260401673&w=2 then
> > only those profiles codec profiles are created which are supported
> > by remote headset/endpoint.
> 
> As discussed before, I don't think the codec configuration should be
> exposed via card profiles. There is need for being able to say "switch
> to A2DP" without specifying the codec.

I strongly agree with this. Separate profiles for each codec is simply not the way to go -- it's horrible for usability.

> It's unclear how priority of the codecs (and their parameter
> permutations) should be configured, but it seems quite clear that a
> "set codec" operation on a specific card would be useful. If the "set
> codec" operation is separate from "set profile" operation, then you no
> doubt will ask how to implement the client API for this new operation,
> and I don't have a very good answer.
> 
> Georg Chini has been working on a new messaging API, which would be a
> good fit for this, but that's stalled (I don't remember if it's waiting
> for review or a new version of the patches). If you don't want to be
> blocked by that, the alternative is to add new "get bluetooth card
> info" and "set bluetooth card a2dp codec" commands to the core protocol
> (the card info command would be for enumerating the available codecs),
> or to add the commands via a new "protocol extension" similar to what
> module-stream-restore, module-device-restore and module-device-manager
> do. I would be fine with either approach.
> 
> Adding new commands to the core protocol would be somewhat simpler, but
> I'm not sure other maintainers are ok with adding bluetooth specific
> functionality to the core protocol.

I don't think adding this to the core is necessarily the best option, but I think this is a separate problem.

The current patchset should, imo, just take a priority-ordered list of codecs as a modarg and use that (we can choose some default if it is not specified, also ideally based on what codecs are supported on the system -- as I've suggested before, I don't want us to depend on the codec implementation, but I can help deal with that as a separate step).

So the modarg approach gives us a static configuration option for people who care about this setting immediately, with a sensible default for most of our users who will not. How we can make this runtime configurable can be figured out separately (for example, with Georg's on-going work).

-- Arun
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss




[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux