All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sonny Sasaka <sonnysasaka@chromium.org>
To: linux-bluetooth@vger.kernel.org
Cc: Sonny Sasaka <sonnysasaka@chromium.org>,
	Archie Pusaka <apusaka@chromium.org>,
	Miao-chen Chou <mcchou@chromium.org>
Subject: [PATCH BlueZ] Queue SetAbsoluteVolume if there is an in-progress one.
Date: Mon,  7 Jun 2021 11:46:16 -0700	[thread overview]
Message-ID: <20210607184616.22051-1-sonnysasaka@chromium.org> (raw)

SetAbsoluteVolume command may receive late response for Target Device
that have high latency processing. In that case we may send the next
SetAbsoluteVolume commands before the previous SetAbsoluteVolume
response is received. This causes the media transport volume to jitter.

The solution in this patch is to not send any SetAbsoluteVolume command
if there is an in-progress one. Instead we should queue this command to
be executed after the in-progress one receives the response.

Reviewed-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>

---
 profiles/audio/avrcp.c | 49 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index ccf34b220..c6946dc46 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -256,6 +256,11 @@ struct avrcp_data {
 	GSList *players;
 };
 
+struct set_volume_command {
+	uint8_t volume;
+	bool notify;
+};
+
 struct avrcp {
 	struct avrcp_server *server;
 	struct avctp *conn;
@@ -275,6 +280,12 @@ struct avrcp {
 	uint8_t transaction;
 	uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
 	struct pending_pdu *pending_pdu;
+	// Whether there is a SetAbsoluteVolume command that is still waiting
+	// for response.
+	bool is_set_volume_in_progress;
+	// If this is non-null, then we need to issue SetAbsoluteVolume
+	// after the current in-progress SetAbsoluteVolume receives response.
+	struct set_volume_command *queued_set_volume;
 };
 
 struct passthrough_handler {
@@ -4252,6 +4263,24 @@ static void target_destroy(struct avrcp *session)
 	g_free(target);
 }
 
+void update_queued_set_volume(struct avrcp *session, uint8_t volume,
+				bool notify)
+{
+	if (!session->queued_set_volume)
+		session->queued_set_volume = g_new0(struct set_volume_command,
+							1);
+	session->queued_set_volume->volume = volume;
+	session->queued_set_volume->notify = notify;
+}
+
+void clear_queued_set_volume(struct avrcp *session)
+{
+	if (!session->queued_set_volume)
+		return;
+	g_free(session->queued_set_volume);
+	session->queued_set_volume = NULL;
+}
+
 static void session_destroy(struct avrcp *session, int err)
 {
 	struct avrcp_server *server = session->server;
@@ -4295,6 +4324,8 @@ static void session_destroy(struct avrcp *session, int err)
 	if (session->browsing_id > 0)
 		avctp_unregister_browsing_pdu_handler(session->browsing_id);
 
+	clear_queued_set_volume(session);
+
 	g_free(session);
 }
 
@@ -4486,6 +4517,8 @@ static gboolean avrcp_handle_set_volume(struct avctp *conn, uint8_t code,
 	struct avrcp_header *pdu = (void *) operands;
 	int8_t volume;
 
+	session->is_set_volume_in_progress = false;
+
 	if (code == AVC_CTYPE_REJECTED || code == AVC_CTYPE_NOT_IMPLEMENTED ||
 								pdu == NULL)
 		return FALSE;
@@ -4495,6 +4528,13 @@ static gboolean avrcp_handle_set_volume(struct avctp *conn, uint8_t code,
 	/* Always attempt to update the transport volume */
 	media_transport_update_device_volume(session->dev, volume);
 
+	if (session->queued_set_volume) {
+		avrcp_set_volume(session->dev,
+					session->queued_set_volume->volume,
+					session->queued_set_volume->notify);
+		clear_queued_set_volume(session);
+	}
+
 	if (player != NULL)
 		player->cb->set_volume(volume, session->dev, player->user_data);
 
@@ -4570,6 +4610,14 @@ int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
 	if (session == NULL)
 		return -ENOTCONN;
 
+	// If there is an in-progress SetAbsoluteVolume, just update the
+	// queued_set_volume. Once the in-progress SetAbsoluteVolume receives
+	// response, it will send the queued SetAbsoluteVolume command.
+	if (session->is_set_volume_in_progress) {
+		update_queued_set_volume(session, volume, notify);
+		return 0;
+	}
+
 	if (notify) {
 		if (!session->target)
 			return -ENOTSUP;
@@ -4589,6 +4637,7 @@ int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
 	pdu->params[0] = volume;
 	pdu->params_len = htons(1);
 
+	session->is_set_volume_in_progress = TRUE;
 	return avctp_send_vendordep_req(session->conn,
 					AVC_CTYPE_CONTROL, AVC_SUBUNIT_PANEL,
 					buf, sizeof(buf),
-- 
2.31.0


             reply	other threads:[~2021-06-07 18:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 18:46 Sonny Sasaka [this message]
2021-06-07 20:15 ` [BlueZ] Queue SetAbsoluteVolume if there is an in-progress one bluez.test.bot
2021-06-07 21:16 ` [PATCH BlueZ] " Luiz Augusto von Dentz
2021-06-07 22:46   ` Marijn Suijten
2021-06-07 23:47     ` Luiz Augusto von Dentz
2021-06-08  9:50       ` Marijn Suijten
2021-06-08 18:37         ` Sonny Sasaka
2021-08-08 10:15           ` Marijn Suijten
2021-08-09 17:46             ` Sonny Sasaka
2021-08-09 20:31               ` Marijn Suijten
2021-08-09 20:49                 ` Sonny Sasaka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210607184616.22051-1-sonnysasaka@chromium.org \
    --to=sonnysasaka@chromium.org \
    --cc=apusaka@chromium.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=mcchou@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.