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
