Re: [PATCH BlueZ v2] audio/avrcp: Determine Absolute Volume support from feature category 2

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

 



Hi Luiz,

On 2021-10-25 10:48:34, Luiz Augusto von Dentz wrote:
> Hi Marijn,
> 
> On Mon, Oct 25, 2021 at 6:42 AM Marijn Suijten
> <marijn.suijten@xxxxxxxxxxxxxx> wrote:
> >
> > Hi Luiz,
> > [..]
> > As far as I'm aware AVRCP 1.3 doesn't define FEATURE_CATEGORY_2 either
> > (but I don't have that spec available to check) so IOP would only find
> > this if it combines v1.3 with that feature bit and then tries to check
> > CAP_EVENTS_SUPPORTED.  But if it does that, it should also find that
> > we're not even checking for the controller supporting FEATURE_CATEGORY_2
> > in the first place, nor are disallowing the controller to send
> > SetAbsoluteVolume.  That's something we should add for sure, even if we
> > don't go ahead with decreasing the minimum version for category-2
> > features below 1.4.
> > I can send a preliminary patch enforcing this if you want.
> 
> So you are saying FEATURE_CATEGORY_2 is not defined in AVRCP 1.3
> either? If the is the case we should probably make it clear on the
> code with a code comment that we will be going to verify it only
> because of Android using it with AVRCP 1.3, but I wonder if there is
> anything in the records that you give us the information that it is
> indeed Android and we should be fine doing such check since AOSP has
> been doing this for a while.

I'm not sure since I don't have access to the 1.3 spec and haven't found
it online in a quick search.  This however makes the most sense since
feature category 2 seems to _only_ concern itself with volume-related
functionality, which are merely SetAbsoluteVolume and
EVENT_VOLUME_CHANGED and introduced only since 1.4.

I wonder if there's anything specific besides the class indicating a
phone factor and the appearance of an avrcp controller with v1.3 but
this feature bit set.

I'll send a followup in two stages: one that introduces a
FEATURE_CATEGORY_2 check around all volume handling, and another that
bumps the requirement for the peer-controller down to v1.3 with clear
comments about AOSP - unless you have better ideas to detect it :)

> > [..]
> >
> > Finally, on the subject of incorrect behaviour and IOP, I found
> > 179ccb936 ("avrcp: Set volume if volume changed event is registered")
> > which also seems counter-intuitive besides going completely against the
> > spec.  It doesn't seem to have gone in through the mailing lists nor
> > discusses the affected device and any potential misbehaviour as a
> > result.  If you're concerned with this patch, is that something you'd
> > like to keep as well?

Anything on this commit?  I'd like to improve the FEATURE_CATEGORY_2
checks and this is quite alarming and conflicting with that.

- Marijn



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux