Hi,
The passthrough command handler - handle_panel_passthrough - ends up responding with response code - ACCEPTED - in below mentioned cases whereas expected response code as per specification is different. Please find the cases and expected response code mentioned below. The document referred for specifying the expected response code is " AV/C Digital Interface Command Set General Specification Version 4.2" which is referred to in AVRCP specification.
1- Command is supported by Bluez but the Media App doesn't support the same e.g. "FAST FORWARD" is supported by Bluez means present in " key_map" table but Android Froyo Media App doesn't support the same.
- As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
2- Command is supported by Bluez and the Media App. But, command can't be executed at this time because of an error e.g. "PLAY" is supported by Bluez and Media App but playback is not possible when Media App receives this command because of an error.
- As per point 6 of section "6.3 AV/C response rules" in above specified document, the expected response code is - REJECTED. Here is the text for point 6 from document: " If the target can not execute the CONTROL, STATUS, or NOTIFY command, and a NOT IMPLEMENTED response would not be required for the command, the target shall return a REJECTED response."
3- Command is not supported by Bluez itself, so doesn't matter whether Media App supports it or not. e.g. " key_map" table is modified to remove "FAST FORWARD".
- As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
Could someone help answering following queries:
- What is the reason for Bluez to respond with response code - ACCEPTED - in these cases? Is this well thought of or just an issue?
- If this is not an issue, it shall be great help if you can provide the reasons for current implementation.
- If this is an issue, is there already a plan to correct it? We need to establish the mechanism wherein Bluez waits for response from client (Media App) whenever necessary (case 1 and 2 mentioned above) before sending response.
Please let me know if you need more information in order to answer the queries.
Thanks.
Best Regards
Sunil Kumar
Hi Sunil,
On Wed, Feb 29, 2012 at 2:20 PM, Kumar, Sunil A <[email protected]> wrote:
> Hi Luiz/Lucas,
>
> Thanks for your inputs.
>
> I agree that situation can be workaround to a great extent using metadata in devices supporting the same. Though, the error codes hold importance in some cases:
>
> - Purely from specification perspective, we are not doing right thing here. But, we can ignore this until we have strong reasons for not doing so :-)
AV/C spec targets completely different system than AVRCP, that why
ipod like devices uses MTP.
> - Consider a Carkit which supports Media Player with TOUCH BUTTONS and still operating with AVRCP 1.0. User presses the Play button to start the music playback but TG could not really start it because of an internal error. In this case, if TG responds with ACCEPTED then I expect Carkit to change Play button to Pause button even though playback has not started (many of the Carkits really will not look at A2DP state to change the play button to pause button) whereas if TG responds with REJECTED then Play button will still stay as Play which is right as well so that user can try again.
The carkit implementations really got this wrong, first there are
several recommendation not do toggling for PLAY/PAUSE, the second and
to me more important is that TG should not be treated as single
purpose player application so using the PLAY to try to start a
specific application just doesn't work, it is the same as hitting play
button on keyboard even Windows doesn't react to that because it
doesn't know what application to open or what to play.
AVRCP 1.4 attempt to fix this with media browsing, although Im not
sure why this is not done using OBEX as it fits much better when
possible dealing with files and browsing directories, so you can
actually select what you want to play.
> - Assumption in Bluez that application supports all commands supported by Bluez is not right as some of these commands are optional and hence, application can choose to not support. And hence, it might be better option to leave the final decision on user (application in this case) but of course, we have complexities in place mentioned by Luiz.
We are assuming that if we are able to translate the command into
Linux input system it will be considered accepted, it is the remote
stack that is trying to be smart and triggering special handling
depending on the response. It also doesn't work given the application
the possibility to tell what commands it supports, because this is
done in runtime depending on what application is in focus, it can even
be a browser accessing youtube.
> Again, as Luiz said effort is really high to change the design. So, not sure if gains are really proportional and hence, please see if we really need to do something here.
I really think this doesn't worth the effort, BlueZ already supports
1.3 and there is plans to support 1.4, Android and others will follow
and carkits will have to catch up as well, so trying to do something
about this for AVRCP 1.0 sounds pretty useless to me.
--
Luiz Augusto von Dentz
Hi Luiz/Lucas,
Thanks for your inputs.
I agree that situation can be workaround to a great extent using metadata in devices supporting the same. Though, the error codes hold importance in some cases:
- Purely from specification perspective, we are not doing right thing here. But, we can ignore this until we have strong reasons for not doing so :-)
- Consider a Carkit which supports Media Player with TOUCH BUTTONS and still operating with AVRCP 1.0. User presses the Play button to start the music playback but TG could not really start it because of an internal error. In this case, if TG responds with ACCEPTED then I expect Carkit to change Play button to Pause button even though playback has not started (many of the Carkits really will not look at A2DP state to change the play button to pause button) whereas if TG responds with REJECTED then Play button will still stay as Play which is right as well so that user can try again.
- Assumption in Bluez that application supports all commands supported by Bluez is not right as some of these commands are optional and hence, application can choose to not support. And hence, it might be better option to leave the final decision on user (application in this case) but of course, we have complexities in place mentioned by Luiz.
Again, as Luiz said effort is really high to change the design. So, not sure if gains are really proportional and hence, please see if we really need to do something here.
Lucas,
Thanks for the patch. I was not able to test it today but will test it tomorrow and send you the feedback.
Thanks.
Best Regards
Sunil Kumar
-----Original Message-----
From: Luiz Augusto von Dentz [mailto:[email protected]]
Sent: Tuesday, February 28, 2012 7:22 PM
To: Lucas De Marchi
Cc: Kumar, Sunil A; [email protected]; Nair, Rashmi G; Khanzode, Prashant; Von Dentz, Luiz
Subject: Re: Response to AVRCP Passthrough Commands.
Hi Lucas,
On Tue, Feb 28, 2012 at 3:05 PM, Lucas De Marchi <[email protected]> wrote:
> Hi Kumar,
>
> * Kumar, Sunil A <[email protected]> [2012-02-28 09:47:20 +0000]:
>
>> Hi,
>>
>> The passthrough command handler - handle_panel_passthrough - ends up responding with response code - ACCEPTED - in below mentioned cases whereas expected response code as per specification is different. Please find the cases and expected response code mentioned below. The document referred for specifying the expected response code is " AV/C Digital Interface Command Set General Specification Version 4.2" which is referred to in AVRCP specification.
>>
>> 1- Command is supported by Bluez but the Media App doesn't support the same e.g. "FAST FORWARD" is supported by Bluez means present in " key_map" table but Android Froyo Media App doesn't support the same.
>> ? ? ? - As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
>
> As per current implementation, the application on top of BlueZ is
> supposed to implement that commands.
We could in theory have the supported_keys as property when registering the MediaPlayer, the problem is that we don't know what Player or application will be in focus to receive the keys.
Btw what would you do if you with not implemented? Make FAST FORWARD unavailable? Bear in mind that you can have multiple players in the system, so the 1:1 map that AV/C and AVRCP uses is not really what BlueZ is target, in other words it is not a TV or microsystem with single mode to work, also note that normally in such case the target is that one that display the error since the controller is just a input device which is exactly how we are treating it by using uinput.
>
>>
>> 2- Command is supported by Bluez and the Media App. But, command can't be executed at this time because of an error e.g. "PLAY" is supported by Bluez and Media App but playback is not possible when Media App receives this command because of an error.
>> ? ? ? - As per point 6 of section "6.3 AV/C response rules" in above specified document, the expected response code is - REJECTED. Here is the text for point 6 from document: " If the target can not execute the CONTROL, STATUS, or NOTIFY command, and a NOT IMPLEMENTED response would not be required for the command, the target shall return a REJECTED response."
Again what would you do with rejected? Does it make any difference if we cannot say what the error is about or display it to the user.
>> 3- Command is not supported by Bluez itself, so doesn't matter whether Media App supports it or not. e.g. " key_map" table is modified to remove "FAST FORWARD".
>> ? ? ? - As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
>
> In this latter case it seems a bug indeed. Does the patch bellow fix
> the issue for you?
Yep that is fine because we know that we will never implement it regardless of what application is running because we could not translate the command to a key press, although Im afraid this makes no difference to the controller.
>>
>> Could someone help answering following queries:
>> ? ? ? - What is the reason for Bluez to respond with response code - ACCEPTED - in these cases? Is this well thought of or just an issue?
>
> The last one is probably a bug. Could you test the patch and see if it
> passes all related tests in PTS?
>
>> ? ? ? - If this is not an issue, it shall be great help if you can provide the reasons for current implementation.
>
> We use uinput here to send the commands. uinput is just like a
> keyboard pressing that "media keys". The problem here is that there's
> no feedback from the application if the command was accepted or not.
>
> Luiz, what do you think?
With uinput we have no feedback no, and if we need one than we have to direct target a single application and it will no longer use a traditional input method to handle those events, so chances are that this will need a lot of effort for basically nothing since the controllers normally cannot anything about it, they are Panel with buttons. The ones with metadata support fix this because then you have a real feedback of the playback, so you know when PLAY failed because the metadata reflect that, same thing with FAST FORWARD not being supported the position will not go forward.
>
>> ? ? ? - If this is an issue, is there already a plan to correct it? We need to establish the mechanism wherein Bluez waits for response from client (Media App) whenever necessary (case 1 and 2 mentioned above) before sending response.
>
> Unless we stop using uinput to use another mechanism to send the
> commands to application, I don't see an easy way to fix it.
Use the metadata.
>
> Regards,
> Lucas De Marchi
>
> ---
> Subject: AVCTP: return not-implemented for unknown passthrough command
>
> diff --git a/audio/avctp.c b/audio/avctp.c index 5bd5db1..e36353d
> 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -235,9 +235,12 @@ static size_t handle_panel_passthrough(struct
> avctp *session,
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?}
>
> - ? ? ? if (key_map[i].name == NULL)
> + ? ? ? if (key_map[i].name == NULL) {
> ? ? ? ? ? ? ? ?DBG("AV/C: unknown button 0x%02X %s",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?operands[0] & 0x7F,
> status);
> + ? ? ? ? ? ? ? *code = AVC_CTYPE_NOT_IMPLEMENTED;
> + ? ? ? ? ? ? ? return 0;
> + ? ? ? }
>
> ?done:
> ? ? ? ?*code = AVC_CTYPE_ACCEPTED;
Patch looks good, can you send a proper one to be applied?
--
Luiz Augusto von Dentz
Hi Ram,
On Tue, Feb 28, 2012 at 12:40 PM, Malovany, Ram <[email protected]> wrote:
> Hi Lucas
>
>> -----Original Message-----
>> From: [email protected] [mailto:linux-bluetooth-
>> [email protected]] On Behalf Of Lucas De Marchi
>> Sent: Tuesday, February 28, 2012 4:20 PM
>> To: [email protected]
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; Lucas De Marchi
>> Subject: [PATCH BlueZ] AVCTP: return not-implemented for unknown passthrough
>>
>> ---
>> ?audio/avctp.c | ? ?5 ++++-
>> ?1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/audio/avctp.c b/audio/avctp.c
>> index 5bd5db1..1d425eb 100644
>> --- a/audio/avctp.c
>> +++ b/audio/avctp.c
>> @@ -235,9 +235,12 @@ static size_t handle_panel_passthrough(struct avctp
>> *session,
>> ? ? ? ? ? ? ? break;
>> ? ? ? }
>>
>> - ? ? if (key_map[i].name == NULL)
>> + ? ? if (key_map[i].name == NULL) {
>> ? ? ? ? ? ? ? DBG("AV/C: unknown button 0x%02X %s",
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? operands[0] & 0x7F, status);
>> + ? ? ? ? ? ? *code = AVC_CTYPE_NOT_IMPLEMENTED;
>> + ? ? ? ? ? ? return 0;
>
> This line here is problematic in the current implementation you will reply with the wrong values at the AVRCP respond packet , when returning 0 you will truncated the Operands( State Flag , Operation Id and Operation Data Length) from the AVRCP reply.
> You need to return with the operand_count this way you will include the full response at the AVRCP profile.
I'm truncating on purpose. AFAIR not-implemented responses have no
operands. See session_cb() function:
packet_size = AVCTP_HEADER_LENGTH + AVC_HEADER_LENGTH;
avctp->cr = AVCTP_RESPONSE;
if (avctp->packet_type != AVCTP_PACKET_SINGLE) {
avc->code = AVC_CTYPE_NOT_IMPLEMENTED;
goto done;
}
...
done:
ret = write(sock, buf, packet_size);
if (ret != packet_size)
goto failed;
return TRUE;
Lucas De Marchi
Hi Lucas
> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Lucas De Marchi
> Sent: Tuesday, February 28, 2012 4:20 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Lucas De Marchi
> Subject: [PATCH BlueZ] AVCTP: return not-implemented for unknown passthrough
>
> ---
> audio/avctp.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/audio/avctp.c b/audio/avctp.c
> index 5bd5db1..1d425eb 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -235,9 +235,12 @@ static size_t handle_panel_passthrough(struct avctp
> *session,
> break;
> }
>
> - if (key_map[i].name == NULL)
> + if (key_map[i].name == NULL) {
> DBG("AV/C: unknown button 0x%02X %s",
> operands[0] & 0x7F, status);
> + *code = AVC_CTYPE_NOT_IMPLEMENTED;
> + return 0;
This line here is problematic in the current implementation you will reply with the wrong values at the AVRCP respond packet , when returning 0 you will truncated the Operands( State Flag , Operation Id and Operation Data Length) from the AVRCP reply.
You need to return with the operand_count this way you will include the full response at the AVRCP profile.
> + }
>
> done:
> *code = AVC_CTYPE_ACCEPTED;
> --
> 1.7.9.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
Malovany Ram
---
audio/avctp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 5bd5db1..1d425eb 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -235,9 +235,12 @@ static size_t handle_panel_passthrough(struct avctp *session,
break;
}
- if (key_map[i].name == NULL)
+ if (key_map[i].name == NULL) {
DBG("AV/C: unknown button 0x%02X %s",
operands[0] & 0x7F, status);
+ *code = AVC_CTYPE_NOT_IMPLEMENTED;
+ return 0;
+ }
done:
*code = AVC_CTYPE_ACCEPTED;
--
1.7.9.2
Hi Lucas,
On Tue, Feb 28, 2012 at 3:05 PM, Lucas De Marchi
<[email protected]> wrote:
> Hi Kumar,
>
> * Kumar, Sunil A <[email protected]> [2012-02-28 09:47:20 +0000]:
>
>> Hi,
>>
>> The passthrough command handler - handle_panel_passthrough - ends up responding with response code - ACCEPTED - in below mentioned cases whereas expected response code as per specification is different. Please find the cases and expected response code mentioned below. The document referred for specifying the expected response code is " AV/C Digital Interface Command Set General Specification Version 4.2" which is referred to in AVRCP specification.
>>
>> 1- Command is supported by Bluez but the Media App doesn't support the same e.g. "FAST FORWARD" is supported by Bluez means present in " key_map" table but Android Froyo Media App doesn't support the same.
>> ? ? ? - As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
>
> As per current implementation, the application on top of BlueZ is
> supposed to implement that commands.
We could in theory have the supported_keys as property when
registering the MediaPlayer, the problem is that we don't know what
Player or application will be in focus to receive the keys.
Btw what would you do if you with not implemented? Make FAST FORWARD
unavailable? Bear in mind that you can have multiple players in the
system, so the 1:1 map that AV/C and AVRCP uses is not really what
BlueZ is target, in other words it is not a TV or microsystem with
single mode to work, also note that normally in such case the target
is that one that display the error since the controller is just a
input device which is exactly how we are treating it by using uinput.
>
>>
>> 2- Command is supported by Bluez and the Media App. But, command can't be executed at this time because of an error e.g. "PLAY" is supported by Bluez and Media App but playback is not possible when Media App receives this command because of an error.
>> ? ? ? - As per point 6 of section "6.3 AV/C response rules" in above specified document, the expected response code is - REJECTED. Here is the text for point 6 from document: " If the target can not execute the CONTROL, STATUS, or NOTIFY command, and a NOT IMPLEMENTED response would not be required for the command, the target shall return a REJECTED response."
Again what would you do with rejected? Does it make any difference if
we cannot say what the error is about or display it to the user.
>> 3- Command is not supported by Bluez itself, so doesn't matter whether Media App supports it or not. e.g. " key_map" table is modified to remove "FAST FORWARD".
>> ? ? ? - As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
>
> In this latter case it seems a bug indeed. Does the patch bellow fix the
> issue for you?
Yep that is fine because we know that we will never implement it
regardless of what application is running because we could not
translate the command to a key press, although Im afraid this makes no
difference to the controller.
>>
>> Could someone help answering following queries:
>> ? ? ? - What is the reason for Bluez to respond with response code - ACCEPTED - in these cases? Is this well thought of or just an issue?
>
> The last one is probably a bug. Could you test the patch and see if it
> passes all related tests in PTS?
>
>> ? ? ? - If this is not an issue, it shall be great help if you can provide the reasons for current implementation.
>
> We use uinput here to send the commands. uinput is just like a keyboard
> pressing that "media keys". The problem here is that there's no feedback
> from the application if the command was accepted or not.
>
> Luiz, what do you think?
With uinput we have no feedback no, and if we need one than we have to
direct target a single application and it will no longer use a
traditional input method to handle those events, so chances are that
this will need a lot of effort for basically nothing since the
controllers normally cannot anything about it, they are Panel with
buttons. The ones with metadata support fix this because then you have
a real feedback of the playback, so you know when PLAY failed because
the metadata reflect that, same thing with FAST FORWARD not being
supported the position will not go forward.
>
>> ? ? ? - If this is an issue, is there already a plan to correct it? We need to establish the mechanism wherein Bluez waits for response from client (Media App) whenever necessary (case 1 and 2 mentioned above) before sending response.
>
> Unless we stop using uinput to use another mechanism to send the
> commands to application, I don't see an easy way to fix it.
Use the metadata.
>
> Regards,
> Lucas De Marchi
>
> ---
> Subject: AVCTP: return not-implemented for unknown passthrough command
>
> diff --git a/audio/avctp.c b/audio/avctp.c
> index 5bd5db1..e36353d 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -235,9 +235,12 @@ static size_t handle_panel_passthrough(struct avctp *session,
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?}
>
> - ? ? ? if (key_map[i].name == NULL)
> + ? ? ? if (key_map[i].name == NULL) {
> ? ? ? ? ? ? ? ?DBG("AV/C: unknown button 0x%02X %s",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?operands[0] & 0x7F, status);
> + ? ? ? ? ? ? ? *code = AVC_CTYPE_NOT_IMPLEMENTED;
> + ? ? ? ? ? ? ? return 0;
> + ? ? ? }
>
> ?done:
> ? ? ? ?*code = AVC_CTYPE_ACCEPTED;
Patch looks good, can you send a proper one to be applied?
--
Luiz Augusto von Dentz
Hi Kumar,
* Kumar, Sunil A <[email protected]> [2012-02-28 09:47:20 +0000]:
> Hi,
>
> The passthrough command handler - handle_panel_passthrough - ends up responding with response code - ACCEPTED - in below mentioned cases whereas expected response code as per specification is different. Please find the cases and expected response code mentioned below. The document referred for specifying the expected response code is " AV/C Digital Interface Command Set General Specification Version 4.2" which is referred to in AVRCP specification.
>
> 1- Command is supported by Bluez but the Media App doesn't support the same e.g. "FAST FORWARD" is supported by Bluez means present in " key_map" table but Android Froyo Media App doesn't support the same.
> - As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
As per current implementation, the application on top of BlueZ is
supposed to implement that commands.
>
> 2- Command is supported by Bluez and the Media App. But, command can't be executed at this time because of an error e.g. "PLAY" is supported by Bluez and Media App but playback is not possible when Media App receives this command because of an error.
> - As per point 6 of section "6.3 AV/C response rules" in above specified document, the expected response code is - REJECTED. Here is the text for point 6 from document: " If the target can not execute the CONTROL, STATUS, or NOTIFY command, and a NOT IMPLEMENTED response would not be required for the command, the target shall return a REJECTED response."
>
> 3- Command is not supported by Bluez itself, so doesn't matter whether Media App supports it or not. e.g. " key_map" table is modified to remove "FAST FORWARD".
> - As per point 5 of section "6.3 AV/C response rules" in above specified document, the expected response code is - NOT IMPLEMENTED. Here is the text for point 5 from document: " If the fields marked in the ck column of the command frame include the unsupported value, the target shall return a NOT IMPLEMENTED response."
In this latter case it seems a bug indeed. Does the patch bellow fix the
issue for you?
>
> Could someone help answering following queries:
> - What is the reason for Bluez to respond with response code - ACCEPTED - in these cases? Is this well thought of or just an issue?
The last one is probably a bug. Could you test the patch and see if it
passes all related tests in PTS?
> - If this is not an issue, it shall be great help if you can provide the reasons for current implementation.
We use uinput here to send the commands. uinput is just like a keyboard
pressing that "media keys". The problem here is that there's no feedback
from the application if the command was accepted or not.
Luiz, what do you think?
> - If this is an issue, is there already a plan to correct it? We need to establish the mechanism wherein Bluez waits for response from client (Media App) whenever necessary (case 1 and 2 mentioned above) before sending response.
Unless we stop using uinput to use another mechanism to send the
commands to application, I don't see an easy way to fix it.
Regards,
Lucas De Marchi
---
Subject: AVCTP: return not-implemented for unknown passthrough command
diff --git a/audio/avctp.c b/audio/avctp.c
index 5bd5db1..e36353d 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -235,9 +235,12 @@ static size_t handle_panel_passthrough(struct avctp *session,
break;
}
- if (key_map[i].name == NULL)
+ if (key_map[i].name == NULL) {
DBG("AV/C: unknown button 0x%02X %s",
operands[0] & 0x7F, status);
+ *code = AVC_CTYPE_NOT_IMPLEMENTED;
+ return 0;
+ }
done:
*code = AVC_CTYPE_ACCEPTED;
Hi Luiz,
Thanks for the details. It clarifies the questions. And agree with you that there is no use to make any change here when we are already moving to AVRCP1.3/1.4.
Hi Lucas,
I was not able to test your patch on latest Bluez version as Android version we are using still makes use of Bluez4.47. But, I made similar changes over 4.47 and I didn't see any error from PTS. So, your patch shall be good to go.
Thanks.
Best Regards
Sunil Kumar
-----Original Message-----
From: Luiz Augusto von Dentz [mailto:[email protected]]
Sent: Wednesday, February 29, 2012 6:53 PM
To: Kumar, Sunil A
Cc: Lucas De Marchi; [email protected]; Nair, Rashmi G; Khanzode, Prashant; Von Dentz, Luiz
Subject: Re: Response to AVRCP Passthrough Commands.
Hi Sunil,
On Wed, Feb 29, 2012 at 2:20 PM, Kumar, Sunil A <[email protected]> wrote:
> Hi Luiz/Lucas,
>
> Thanks for your inputs.
>
> I agree that situation can be workaround to a great extent using metadata in devices supporting the same. Though, the error codes hold importance in some cases:
>
> - Purely from specification perspective, we are not doing right thing
> here. But, we can ignore this until we have strong reasons for not
> doing so :-)
AV/C spec targets completely different system than AVRCP, that why ipod like devices uses MTP.
> - Consider a Carkit which supports Media Player with TOUCH BUTTONS and still operating with AVRCP 1.0. User presses the Play button to start the music playback but TG could not really start it because of an internal error. In this case, if TG responds with ACCEPTED then I expect Carkit to change Play button to Pause button even though playback has not started (many of the Carkits really will not look at A2DP state to change the play button to pause button) whereas if TG responds with REJECTED then Play button will still stay as Play which is right as well so that user can try again.
The carkit implementations really got this wrong, first there are several recommendation not do toggling for PLAY/PAUSE, the second and to me more important is that TG should not be treated as single purpose player application so using the PLAY to try to start a specific application just doesn't work, it is the same as hitting play button on keyboard even Windows doesn't react to that because it doesn't know what application to open or what to play.
AVRCP 1.4 attempt to fix this with media browsing, although Im not sure why this is not done using OBEX as it fits much better when possible dealing with files and browsing directories, so you can actually select what you want to play.
> - Assumption in Bluez that application supports all commands supported by Bluez is not right as some of these commands are optional and hence, application can choose to not support. And hence, it might be better option to leave the final decision on user (application in this case) but of course, we have complexities in place mentioned by Luiz.
We are assuming that if we are able to translate the command into Linux input system it will be considered accepted, it is the remote stack that is trying to be smart and triggering special handling depending on the response. It also doesn't work given the application the possibility to tell what commands it supports, because this is done in runtime depending on what application is in focus, it can even be a browser accessing youtube.
> Again, as Luiz said effort is really high to change the design. So, not sure if gains are really proportional and hence, please see if we really need to do something here.
I really think this doesn't worth the effort, BlueZ already supports
1.3 and there is plans to support 1.4, Android and others will follow and carkits will have to catch up as well, so trying to do something about this for AVRCP 1.0 sounds pretty useless to me.
--
Luiz Augusto von Dentz
Hi Lucas,
On Tue, Feb 28, 2012, Lucas De Marchi wrote:
> ---
> audio/avctp.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/audio/avctp.c b/audio/avctp.c
> index 5bd5db1..1d425eb 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -235,9 +235,12 @@ static size_t handle_panel_passthrough(struct avctp *session,
> break;
> }
>
> - if (key_map[i].name == NULL)
> + if (key_map[i].name == NULL) {
> DBG("AV/C: unknown button 0x%02X %s",
> operands[0] & 0x7F, status);
> + *code = AVC_CTYPE_NOT_IMPLEMENTED;
> + return 0;
> + }
>
> done:
> *code = AVC_CTYPE_ACCEPTED;
Applied. Thanks.
Johan