Hi Marek,
On Wed, 21 Oct 2020 at 16:02, Marek Czerski <ma.czerski@xxxxxxxxx> wrote:
>
> Hi Marijn,
>
> Regarding the AVRCP version reported by the phone. There is something
> weird going on on android regarding avrcp version and mac addresses
> and absolute volume. I discovered couple of whings:
> 1. galaxy S7 will blacklist some mac addresses to not use absolute
> volume for that device (checked it with "adb shell dumpsys
> bluetooth_manager"). Changing mac addres of the device to something
> other allows for absolute volume to work. For both cases it reports
> avrcp version 1.4.
There is indeed a database with traits based on mac-addresses [1].
> 2. xiaomi redmi note 4 will report different avrcp version depending
> on mac address. version 1.3 for mac address that galaxy s7 blacklists
> and 1.6 for mac address that galaxy s7 does not blacklist. But in both
> cases the absolute volume does not work ...
You confirmed earlier that besides this versioning issue there's also
an initialization issue with supported_events. I assume this test was
without flipping the _init calls around?
> 3. xiaomi redmi 8 will not even report that it has "A/V_RemoteControl"
> (UUID 0x110E), it just reports that it has only
> "A/V_RemoteControlTarget" (UUID 0x110C) but still it can control
> volume on my yamaha av receiver and sony headphones and receive volume
> updates from them. Very, very wierd.
That's not really surprising to me, in fact it is slightly weird that
other phones report the A/V_RemoteControl profile. A phone will always
be the audio source (thus the RemoteControlTarget), never the sink.
(Unless applying some hacks to libfluoride: it is possible to use an
older phone as A2DP sink - for example to connect to a receiver at the
other side of the room. I would have enabled it on all my phones if it
didn't break A2DP source capabilities at the same time... Maybe
Gabeldorsche addresses that)
> For the record, to check the avrcp version of the phone I use sdptool
> browse command (and to be 100% sure I check also the raw communication
> on wireshark). For example this is one of the records reported for
> xiaomi redmi note 4 and avrcp version 1.6:
> Service RecHandle: 0x10007
> Service Class ID List:
> "AV Remote" (0x110e)
> "AV Remote Controller" (0x110f)
> Protocol Descriptor List:
> "L2CAP" (0x0100)
> PSM: 23
> "AVCTP" (0x0017)
> uint16: 0x0104
> Profile Descriptor List:
> "AV Remote" (0x110e)
> Version: 0x0106
> If there is something more I can do to help please let me know. For
> example I can send hci snoop logs for specific cases etc.
I will send a _preliminary_ patch to this thread that should resolve
both the versioning and ordering issue as I've finally figured a way
to check whether the session associated with an AVRCP command is
running as source or sink. It is not finalized yet so please don't be
too hard on it :)
> wt., 20 paź 2020 o 15:12 Marek Czerski <ma.czerski@xxxxxxxxx> napisał(a):
> >
> > Hi Marijn,
> >
> > wt., 20 paź 2020 o 11:11 Marijn Suijten <marijns95@xxxxxxxxx> napisał(a):
> > >
> > > Hi Marek,
> > >
> > > (Apologies for sending this to you twice; missed the reply-all button
> > > so this didn't end up on the mailing list)
> > >
> > > On Tue, 20 Oct 2020 at 09:31, Marek Czerski <ma.czerski@xxxxxxxxx> wrote:
> > > >
> > > > Hi Marijn,
> > > >
> > > > pon., 19 paź 2020 o 21:53 Marijn Suijten <marijns95@xxxxxxxxx> napisał(a):
> > > > >
> > > > > Hi Luiz, Marek,
> > > > >
> > > > > On Mon, 19 Oct 2020 at 18:47, Luiz Augusto von Dentz
> > > > > <luiz.dentz@xxxxxxxxx> wrote:
> > > > > >
> > > > > > Hi Marek, Marijn,
> > > > > >
> > > > > > On Mon, Oct 19, 2020 at 9:06 AM Marek Czerski <ma.czerski@xxxxxxxxx> wrote:
> > > > > > >
> > > > > > > Hi Marijn,
> > > > > > >
> > > > > > > pon., 19 paź 2020 o 16:16 Marijn Suijten <marijns95@xxxxxxxxx> napisał(a):
> > > > > > > >
> > > > > > > > Hi Marek,
> > > > > > > >
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > I was looking into, so called, absolute volume control that was
> > > > > > > > > introduced in AVRCP v1.4. What I want to achieve is to send audio from
> > > > > > > > > android smartphone to linux based device running bluez and make the
> > > > > > > > > volume control on the smartphone side to control the volume on the
> > > > > > > > > device. So the device is the a2dp sink + avrcp CT/TG and the phone is
> > > > > > > > > a2dp source + avrcp CT/TG.
> > > > > > > > >
> > > > > > > > > I assume that if all is working correctly then on the dbus the Volume
> > > > > > > > > property of the org.bluez.MediaTransport1 will be changed by the
> > > > > > > > > volume control of the phone and changes made to this property from the
> > > > > > > > > device would propagate to the phone volume slider.
> > > > > > > > >
> > > > > > > > > This is not happening and what I believe is the cause is that
> > > > > > > > > AVRCP_EVENT_VOLUME_CHANGED event registration request sent from the
> > > > > > > > > phone is rejected by the bluez. I can see that on the wireshark snoop
> > > > > > > > > from the device's bluetooth adapter. And on wireshark I see that
> > > > > > > > > AVRCP_EVENT_VOLUME_CHANGED event registration is sent by the phone
> > > > > > > > > before bluez initializes session->supported_events variable (not
> > > > > > > > > really sure about that). I think that this rejection makes the phone
> > > > > > > > > to not send SetAbsoluteVolume commands to the device on volume change.
> > > > > > > >
> > > > > > > > I looked into the same issue earlier this year, see
> > > > > > > > 20200118194841.439036-1-marijns95@xxxxxxxxx [1]. The gist of it is that BlueZ
> > > > > > > > bases supported_events solely on the remote AVRCP controller version, which
> > > > > > > > Android sets to 1.3 when it is a media source [2]. This version is not
> > > > > > > > relevant in your use-case because the Android phone is the AVRCP target while
> > > > > > > > blueZ is the controller.
> > > > > > > >
> > > > > > >
> > > > > > > I didn't tested Your patch but after looking at the code it seems that
> > > > > > > just applying Your patch would solve my problem.
> > > > >
> > > > > It should, even though I don't immediately see how swapping
> > > > > target/controller_init as per your initial email solves the problem.
> > > > >
> > > >
> > > > The sequence of events is important here. The
> > > > session->supported_events must be initialised (which is done in
> > > > target_init) before first AVRCP_REGISTER_NOTIFICATION command from the
> > > > remote side controller. If supported_events is not initialised and
> > > > AVRCP_REGISTER_NOTIFICATION arrives the
> > > > avrcp_handle_register_notification function rejects the event
> > > > registration. And this is what I see in wireshark.
> > >
> > > That's interesting and means we are dealing with two distinct problems
> > > here. No matter what order I flip controller/target_init in
> > > AVRCP_REGISTER_NOTIFICATION is always received after both functions
> > > complete. handle_vendordep_pdu is registered before these
> > > initialization functions making this possible though. I wonder if PDUs
> > > are dropped when the callback is registered later, after init. Perhaps
> > > controller/target_init need to be split in two, a static
> > > initialization part based on the remote profile version followed by
> > > registering this callback and finally functions that send commands to
> > > the remote like avrcp_get_capabilities.
> > >
> > > > And in my case, calling target_init earlier (the is before
> > > > controller_init) solves the problem because supported_events is
> > > > already initialised at the time the AVRCP_REGISTER_NOTIFICATION
> > > > arrives.
> > >
> > > That means your phone reports an AVRCP remote version of at least
> > > 0x104, otherwise supported_events would not contain
> > > AVRCP_EVENT_VOLUME_CHANGED. Can you validate this in wireshark or
> > > DBG("%p version 0x%04x", ...) in the logs? I'm seeing:
> > >
> > > profiles/audio/avrcp.c:target_init() 0x5614a2cbab70 version 0x0103
> > >
> >
> > Yes, I get this in my logs when connecting from samsung galaxy S7:
> > profiles/audio/avrcp.c:target_init() 0x1103758 version 0x0104
> > profiles/audio/avrcp.c:controller_init() 0x1112c18 version 0x0104
> >
> > > Either way the split Luiz and I mentioned mean one variant of
> > > supported_events (the one relevant for registering
> > > AVRCP_EVENT_VOLUME_CHANGED) is initialized in controller_init.
> > >
> > > > But I think that the problem is more general. There is some gap
> > > > between AVCTP connection establishment and BlueZ readiness to handle
> > > > commands from the remote side. There could be devices that send
> > > > commands during this gap and those devices would not work correctly
> > > > with BlueZ despite the fact that they are consistent with the
> > > > specification (or not ?).
> > > >
> > > > > > > Regarding avrcp version, in android there is developer option to set
> > > > > > > avrcp version. For example my Xiaomi redmi 8 (android 10) reports
> > > > > > > version according to this setting, but samsung galaxy s7 (android 8)
> > > > > > > always report version 1.4 regardless of this setting.
> > > > >
> > > > > I think this is the version used for the AVRCP Remote Target [1]. The
> > > > > version linked above, the one that is causing problems here is that of
> > > > > the AVRCP controller.
> > > > >
> > > > > There have always been problems changing these settings in developer
> > > > > options - sometimes they work, sometimes they don't. The system has
> > > > > been overhauled for Android 11 and is now much more consistent though
> > > > > avrcp still runs through a property. If I remember correctly
> > > > > restarting bluetooth or re-adding a device (in case of SDP caching) is
> > > > > the only solution.
> > > > >
> > > > > > We need to rework a little bit how the controller/target works, this
> > > > > > roles are actually supposed to be interpreted as client/server and
> > > > > > much like GATT they can be used simultaneously, so we need a target
> > > > > > versions and a controller version and independent supported events.
> > > > >
> > > > > Yes, I think that's the change we proposed last time and are proposing
> > > > > now. Unfortunately relevant functions need to know if the local
> > > > > (BlueZ) end is operating as target or controller for that particular
> > > > > operation.
> > > > >
> > > > > > That said if the remote side controller version does not indicate 1.4
> > > > > > or later we obviously can't support absolute volume control as that is
> > > > > > reserved in earlier versions.
> > > > >
> > > > > That depends on the way audio is flowing. When the local (BlueZ) end
> > > > > is the sink (rendering the audio) meaning it takes the role of AVRCP
> > > > > controller (the situation in the mail from Marek), the remote must be
> > > > > the AVRCP target and hence its controller version is irrelevant.
> > > > > Likewise when BlueZ is the audio source (AVRCP target), it should only
> > > > > care about the remote controller profile and version.
> > > > > But that is exactly what you described above: BlueZ needs to attain
> > > > > separation between handling notification registrations (and perhaps
> > > > > other code) as a controller and target separately.
> > > > >
> > > >
> > > > I don't know if I'm interpreting your words correctly but I think that
> > > > You make an assumption that a2dp sink must be avrcp CT and a2dp source
> > > > is avrcp TG.
> > > > In case of the absolute volume control it is not true. For absolute
> > > > volume control to work it is the other way around which makes both
> > > > sides be CT and TG at the same time.
> > >
> > > Quite the contrary, in my previous mail I attempted to explain
> > > multiple times that the CT/TG designation in the spec is used for
> > > initiator and target respectively, not the AVRCP controller and target
> > > profile. Indeed, depending on the transaction either side (controller
> > > or target) can be the initiator (CT). In other words CT and TG are
> > > _not_ abbreviations for the controller and target profile.
> > >
> > > > a2dp sink is avrcp CT for
> > > > controlling playback and a2dp source is avrcp CT for setting the
> > > > volume. It is the a2dp source that is sending
> > > > AVRCP_SET_ABSOLUTE_VOLUME commands.
> > >
> > > Correct, yet the A2DP sink will always take on the AVRCP controller
> > > profile where the phone being the A2DP source takes the AVRCP target
> > > profile.
> > >
> > > > > > > > It was decided in that mail thread to split supported_events in two; one based
> > > > > > > > on the external controller version (when BlueZ operates as target it'll
> > > > > > > > validate incoming notification registrations) and the other based on what BlueZ
> > > > > > > > currently supports as controller.
> > > > > > > >
> > > > > > > > The second check might not be all too relevant and is already covered by the
> > > > > > > > switch-case; perhaps it makes more sense to base this check on the external
> > > > > > > > target version, and again validate whether we expect to receive that particular
> > > > > > > > notification registration?
> > > > > > > >
> > > > > > > > Both checks together implicitly validate what BlueZ supports locally in its
> > > > > > > > role of controller or target, as remote_{target,controller}_supported_events
> > > > > > > > (anticipated names of the new members replacing supported_events) will only be
> > > > > > > > set to events that BlueZ is able to emit.
> > > > > > > >
> > > > > > >
> > > > > > > One thing is not clear for me, what is the purpose of the
> > > > > > > supported_events ? It is used in two places:
> > > > > > > First is the avrcp_handle_register_notification function. If the
> > > > > > > remote side want to register itself for specific event notification it
> > > > > > > does not matter what version of avrcp that remote side supports. If it
> > > > > > > ask for specific event it clearly support that event.
> > > > >
> > > > > Looking at commit 4ae6135 that introduced this check it was apparently
> > > > > causing crashes without. I can only guess that devices were sending
> > > > > vendor-specific notification requests, assuming implementations are
> > > > > supposed to filter out anything beyond what their reported version
> > > > > supports? Either way input validation is still a sane thing to do.
> > > > >
> > > > > > > Second is in avrcp_handle_get_capabilities in CAP_EVENTS_SUPPORTED
> > > > > > > case. Does it matter if local side reply with events that are not
> > > > > > > supported in the version of avrcp supported by the remote side ?
> > > > >
> > > > > I guess it is sane here to not report capabilities the CT should -
> > > > > given it's version - not know about. I could not find anything in the
> > > > > 1.6.2 spec mandating this though, perhaps Luiz knows.
> > > > >
> > > > > > As I said we need to split the supported events to
> > > > > > ct(client)/tg(server) to avoid interpreting them as the same, it is
> > > > > > very odd that the remote would have different versions for each role
> > > > >
> > > > > Yeah, Android exposes different versions for its AVRCP Remote and
> > > > > AVRCP Remote Target.
> > > > > This is slightly confusing though: we discussed earlier that CT and TG
> > > > > is about the initiator and receiver respectively, with no direct
> > > > > mapping to AVRCP remote and target as either side can be the initiator
> > > > > (sender) of a command. Correct?
> > > > > In this specific case a notification registration will always be
> > > > > initiated by a CT and fulfilled by the TG, no matter whether it is an
> > > > > AVRCP remote or target.
> > > > >
> > > > > > but it looks like this is happening in this case although it is work
> > > > > > confirming if the CT version if in fact 1.3 as well we cannot enable
> > > > > > absolute volume control as that is not supported by that version, what
> > > > > > we can perhaps is to detect if SetAbsoluteVolume control is used then
> > > > > > update the events for the session.
> > > > >
> > > > > Again, in the specific case of this issue it is fine if the CT
> > > > > (initiator, the Android phone, supposed to behave like the target as
> > > > > it is the audio source) reports a controller version of 1.3 as we are
> > > > > only concerned about the target version, which will be the one
> > > > > registering for EVENT_VOLUME_CHANGED notifications from the
> > > > > sink/controller (BlueZ). Android always reports the AVRCP Remote
> > > > > Target as 1.4 or higher based on developer settings [1].
> > > > >
> > > > > > > > Unfortunately my ramblings in that mail shadowed an important question: how to
> > > > > > > > determine in avrcp_handle_register_notification whether BlueZ is running as
> > > > > > > > controller or target? set_volume in transport.c derives this from
> > > > > > > > transport->source_watch but there seems to be no easy access to the
> > > > > > > > accompanying transport in avrcp_handle_register_notification. With this
> > > > > > > > question answered I'll be able to update and resubmit the original patch.
> > > > > > > >
> > > > > > > > > To test my theory i changed the session_init_control function in the
> > > > > > > > > profiles/audio/avrcp.c to call first target_init and then
> > > > > > > > > controller_init. This caused the AVRCP_EVENT_VOLUME_CHANGED event not
> > > > > > > > > been rejected and the volume control from the phone works as expected.
> > > > > > > > >
> > > > > > > > > After reading AVRCP specification I did not find any reason for the CT
> > > > > > > > > on the phone side not to send event registration immediately after the
> > > > > > > > > AVCTP connection establishment. So I believe that bluez should not
> > > > > > > > > reject event registration in this case.
> > > > > > > > >
> > > > > > > > > Best Regards,
> > > > > > > > > Marek Czerski
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Marijn Suijten
> > > > > > > >
> > > > > > > > [1]: https://marc.info/?l=linux-bluetooth&m=157937699001093
> > > > > > > > [2]: https://android.googlesource.com/platform/system/bt/+/android-11.0.0_r4/bta/av/bta_av_main.cc#761
> > > > > > >
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Marek Czerski
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > > P.S.: I hope it is fine to respond to two emails in one, seems like
> > > > > that will get confusing real quick if depth increases.
> > > > >
> > > > > - Marijn
> > > > >
> > > > > [1]: https://android.googlesource.com/platform/system/bt/+/android-11.0.0_r4/bta/av/bta_av_main.cc#612
> > > >
> > > > Best regards,
> > > > Marek
> > >
> > > - Marijn
> >
> >
> >
> > --
> > mgr inż. Marek Czerski
> > +48 696 842 686
>
>
>
> --
> mgr inż. Marek Czerski
> +48 696 842 686
- Marijn
[1]: https://android.googlesource.com/platform/system/bt/+/master/device/include/interop_database.h
