Hi,
I am working on AVRCP for Chromium OS using MediaPlayer dbus API and
noticed a problem while testing the media buttons on various BT
speaker/headsets. In profile/audio/avrcp.c there is a check for
supported feature before an avrcp_player is created:
/* Only create player if category 1 is supported */
if (!(controller->features & AVRCP_FEATURE_CATEGORY_1))
return;
The SDP record of my BT speaker has feature ==
AVRCP_FEATURE_CATEGORY_2 (monitor/amplifier). My understanding is that
this feature is associated with volume changed event and
SetAbsoluteVolume command.
But with this check, volume changed events are unable to pass up to
registered player app in my case. I tried commented out these few
lines and rebuild bluetoothd, after that volume change events works
perfect.
What is the reason to create player only when it supports category 1 feature?
Thanks,
Hsin-yu
Hi Hsin-yu,
On Thu, Jan 21, 2016 at 9:52 AM, Hsin-yu Chao <[email protected]> wrote:
> Hi Luiz,
> Thanks for the comments. I am testing with this UE-mini Boom speaker.
> http://the-gadgeteer.com/2013/11/22/ue-mini-boom-portable-bluetooth-speaker-review/
>
> This BT speaker has two buttons 'Volume Up' and 'Volume Down', but
> lack of player related buttons, like next/prev track,
> shuffle/play/pause. I think that is why it advertised to support only
> category 2 feature.
Yep, this might explain why we haven't seem this before.
> Looking at audio/avrcp.c, set_volume is one of the avrcp_player_cb
> callback functions, plus that avrcp_volume_changed() checks for the
> existence of an avrcp_player struct before handling the event. Seems
> that avrcp_player structure is designed to cover both category 1 and
> category 2 features. Maybe we should change the condition to if
> (!(controller->features & (AVRCP_FEATURE_CATEGORY_1 |
> AVRCP_FEATURE_CATEGORY_2))) instead? So that the set volume cmd and
> volume change event can work correctly on those peripherals who has
> volume button only.
Yep, this might work, we could also create the player object but not
the D-Bus interface which I think for the original idea so application
would not have the false impression there is a player in the other end
so something like this should work:
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 7b60012..076766d 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -3843,12 +3843,11 @@ static void controller_init(struct avrcp *session)
btd_service_connecting_complete(service, 0);
/* Only create player if category 1 is supported */
- if (!(controller->features & AVRCP_FEATURE_CATEGORY_1))
- return;
-
- player = create_ct_player(session, 0);
- if (player == NULL)
- return;
+ if (controller->features & AVRCP_FEATURE_CATEGORY_1) {
+ player = create_ct_player(session, 0);
+ if (player == NULL)
+ return;
+ }
if (controller->version < 0x0103)
return;
Hi Luiz,
Thanks for the comments. I am testing with this UE-mini Boom speaker.
http://the-gadgeteer.com/2013/11/22/ue-mini-boom-portable-bluetooth-speaker-review/
This BT speaker has two buttons 'Volume Up' and 'Volume Down', but
lack of player related buttons, like next/prev track,
shuffle/play/pause. I think that is why it advertised to support only
category 2 feature.
Looking at audio/avrcp.c, set_volume is one of the avrcp_player_cb
callback functions, plus that avrcp_volume_changed() checks for the
existence of an avrcp_player struct before handling the event. Seems
that avrcp_player structure is designed to cover both category 1 and
category 2 features. Maybe we should change the condition to if
(!(controller->features & (AVRCP_FEATURE_CATEGORY_1 |
AVRCP_FEATURE_CATEGORY_2))) instead? So that the set volume cmd and
volume change event can work correctly on those peripherals who has
volume button only.
I will send a patch for that later.
Hsin-yu
On Wed, Jan 20, 2016 at 11:59 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Hsin-yu,
>
> On Tue, Jan 19, 2016 at 2:06 PM, Hsin-yu Chao <[email protected]> wrote:
>> Hi,
>> I am working on AVRCP for Chromium OS using MediaPlayer dbus API and
>> noticed a problem while testing the media buttons on various BT
>> speaker/headsets. In profile/audio/avrcp.c there is a check for
>> supported feature before an avrcp_player is created:
>>
>> /* Only create player if category 1 is supported */
>> if (!(controller->features & AVRCP_FEATURE_CATEGORY_1))
>> return;
>
> Category 1 is player category thus why we check it.
>
>> The SDP record of my BT speaker has feature ==
>> AVRCP_FEATURE_CATEGORY_2 (monitor/amplifier). My understanding is that
>> this feature is associated with volume changed event and
>> SetAbsoluteVolume command.
>
> And GetCapabilities as well since otherwise we cannot subscribe to
> volume changes.
>
>> But with this check, volume changed events are unable to pass up to
>> registered player app in my case. I tried commented out these few
>> lines and rebuild bluetoothd, after that volume change events works
>> perfect.
>>
>> What is the reason to create player only when it supports category 1 feature?
>
> I don't recall now but I guess all the headset we encountered had this
> feature bit set, note that this is the category of a target record but
> perhaps it is valid after all. Btw, is this a BT Speaker of yours a
> device on the market already?
>
>
> --
> Luiz Augusto von Dentz
Hi Hsin-yu,
On Tue, Jan 19, 2016 at 2:06 PM, Hsin-yu Chao <[email protected]> wrote:
> Hi,
> I am working on AVRCP for Chromium OS using MediaPlayer dbus API and
> noticed a problem while testing the media buttons on various BT
> speaker/headsets. In profile/audio/avrcp.c there is a check for
> supported feature before an avrcp_player is created:
>
> /* Only create player if category 1 is supported */
> if (!(controller->features & AVRCP_FEATURE_CATEGORY_1))
> return;
Category 1 is player category thus why we check it.
> The SDP record of my BT speaker has feature ==
> AVRCP_FEATURE_CATEGORY_2 (monitor/amplifier). My understanding is that
> this feature is associated with volume changed event and
> SetAbsoluteVolume command.
And GetCapabilities as well since otherwise we cannot subscribe to
volume changes.
> But with this check, volume changed events are unable to pass up to
> registered player app in my case. I tried commented out these few
> lines and rebuild bluetoothd, after that volume change events works
> perfect.
>
> What is the reason to create player only when it supports category 1 feature?
I don't recall now but I guess all the headset we encountered had this
feature bit set, note that this is the category of a target record but
perhaps it is valid after all. Btw, is this a BT Speaker of yours a
device on the market already?
--
Luiz Augusto von Dentz