[BlueZ,v4,0/3] audio/avrcp: Determine Absolute Volume support from feature category 2

Message

Marijn Suijten Oct. 5, 2024, 9:43 p.m. UTC

Apologies for the very long delay to send v4.  As it turns out review comments
initially failed to reach my inbox [1], and I haven't pursued these patches
afterwards until receiving yet another ping about it on an ancient pulseaudio
MR.

Note that I'm not entirely satisfied with the much longer `if` conditionals
that are increasingly hard to read.  Suggestions are welcome, such as confirming
whether factoring them out into local `bool` variables is desired.

This series can still be tested on recent Android phones by setting `Bluetooth
AVRCP version` to 1.3 in developer settings, followed by **repairing** the phone
to a machine running BlueZ to refresh SDP records.  Absolute volume control
should work unless `VolumeVersion` is set to `false`, resulting in useful error
messages instead.

Keep in mind that this `VolumeVersion` property only applies to when the machine
running BlueZ is "in control of" the volume (i.e. we are the sink) and receiving
SetAbsoluteVolume commands from the peer (and sending EVENT_VOLUME_CHANGED
*to* the peer).  This is because Android appears to use AVRCP 1.4 or higher
in its TG profile (our `session->controller->version`) which should succeed
the version check (and not in the first place because phones generally have
their "audio sink" functionality disabled to not confuse various bidirectional
audio/ bluetooth stacks... unless it's "Android Auto" where it is the other
way around).

[1]: https://marc.info/?l=linux-bluetooth&m=168548631319097&w=2

Changes since v3:
- Shortened `[AVRCP]` configuration parameter names, now `VolumeWithoutTarget`
  and `VolumeVersion`.
- Allow disabling new "strict category-2 checking" from v3 behind a new
  `VolumeCategory` (default `true`) parameter;
- Mentioned in main.conf that `VolumeWithoutTarget` (from a strange patch
  further described below) also skips the AVRCP 1.4 version requirement;
- Rebased 1.5 year old patches.

v3: https://marc.info/?l=linux-bluetooth&m=167849515409848&w=2

Changes since v2:

- New patch to guard target-less SetAbsoluteVolume calls with main.conf
  config;
- New patch to more strictly require category-2 _and_ peer CT/TG 1.4;
- Use main.conf to relax 1.4 version constraint for TGs registering
  AbsVol notifications;

v2: https://marc.info/?l=linux-bluetooth&m=163519566912788&w=2

Note that I still misunderstand commit 179ccb936 ("avrcp: Set volume if volume
changed event is registered").  If we're planning on sending a SetAbsoluteVolume
command to the (inexistant...) TG profile on our peer, why would it check
whether the CT on the peer previously registered for EVENT_VOLUME_CHANGED?  In
that case the peer seems to assume that we know/control the volume, and have to
*reply to the event* instead?

Marijn Suijten (3):
  audio/avrcp: Guard SetAbsoluteVolume without target behind config
    value
  audio/avrcp: Only allow absolute volume call/event on category-2 peers
  audio/avrcp: Determine Absolute Volume support from feature category 2

 profiles/audio/avrcp.c | 57 ++++++++++++++++++++++++++++++++++++------
 src/btd.h              |  7 ++++++
 src/main.c             | 26 +++++++++++++++++++
 src/main.conf          | 20 +++++++++++++++
 4 files changed, 103 insertions(+), 7 deletions(-)