linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Archie Pusaka <apusaka@google.com>
To: linux-bluetooth <linux-bluetooth@vger.kernel.org>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Archie Pusaka <apusaka@chromium.org>, Yu Liu <yudiliu@google.com>
Subject: [Bluez PATCH v4 2/2] audio/transport: supply volume on transport init
Date: Thu, 23 Jul 2020 14:22:50 +0800	[thread overview]
Message-ID: <20200723142128.Bluez.v4.2.Ic16589fde45fac0c496dfca2fa27672059114c3b@changeid> (raw)
In-Reply-To: <20200723142128.Bluez.v4.1.I667fa0ebcc3056a21c22fdaf476a56dd72aff38d@changeid>

From: Archie Pusaka <apusaka@chromium.org>

Sometimes the response of RegisterNotification for volume change
event came before we create the transport for the corresponding
device. If that happens, the volume will be stuck to an
uninitialized invalid value. The property Volume of
MediaTransport1 will also be left unaccessible.

This patch supplies the initial volume when creating a new
transport. The value is obtained from the media_player object.
However, since the avrcp session might not be created by the time
the transport is created, we also try to initialize the volume
when creating avrcp session.

Reviewed-by: Yu Liu <yudiliu@google.com>
---

Changes in v4: None
Changes in v3:
-Add missing library

Changes in v2:
-Get the volume from media_player instead of from separate list

 profiles/audio/avrcp.c     | 27 ++++++++++++++++-
 profiles/audio/avrcp.h     |  2 ++
 profiles/audio/media.c     | 60 +++++++++++++++++++++++++++++---------
 profiles/audio/media.h     |  2 ++
 profiles/audio/transport.c |  2 +-
 5 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index d2d1d9dae..0ea66c671 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -64,6 +64,7 @@
 #include "avctp.h"
 #include "avrcp.h"
 #include "control.h"
+#include "media.h"
 #include "player.h"
 #include "transport.h"
 
@@ -1701,7 +1702,6 @@ static uint8_t avrcp_handle_request_continuing(struct avrcp *session,
 	if (pending->pdu_id != pdu->params[0])
 		goto err;
 
-
 	len = 0;
 	pending->attr_ids = player_fill_media_attribute(player,
 							pending->attr_ids,
@@ -4037,8 +4037,12 @@ static void target_init(struct avrcp *session)
 
 	player = g_slist_nth_data(server->players, 0);
 	if (player != NULL) {
+		int8_t init_volume;
 		target->player = player;
 		player->sessions = g_slist_prepend(player->sessions, session);
+
+		init_volume = media_player_get_device_volume(session->dev);
+		media_transport_update_device_volume(session->dev, init_volume);
 	}
 
 	session->supported_events |= (1 << AVRCP_EVENT_STATUS_CHANGED) |
@@ -4476,6 +4480,27 @@ int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
 					avrcp_handle_set_volume, session);
 }
 
+struct avrcp_player *avrcp_get_target_player_by_device(struct btd_device *dev)
+{
+	struct avrcp_server *server;
+	struct avrcp *session;
+	struct avrcp_data *target;
+
+	server = find_server(servers, device_get_adapter(dev));
+	if (server == NULL)
+		return NULL;
+
+	session = find_session(server->sessions, dev);
+	if (session == NULL)
+		return NULL;
+
+	target = session->target;
+	if (target == NULL)
+		return NULL;
+
+	return target->player;
+}
+
 static int avrcp_connect(struct btd_service *service)
 {
 	struct btd_device *dev = btd_service_get_device(service);
diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
index 026a820f5..702d57442 100644
--- a/profiles/audio/avrcp.h
+++ b/profiles/audio/avrcp.h
@@ -116,3 +116,5 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
 
 size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
 size_t avrcp_browsing_general_reject(uint8_t *operands);
+
+struct avrcp_player *avrcp_get_target_player_by_device(struct btd_device *dev);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index d4d58ec86..02bf82a49 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -239,6 +239,20 @@ static void media_endpoint_exit(DBusConnection *connection, void *user_data)
 	media_endpoint_remove(endpoint);
 }
 
+static struct media_adapter *find_adapter(struct btd_device *device)
+{
+	GSList *l;
+
+	for (l = adapters; l; l = l->next) {
+		struct media_adapter *adapter = l->data;
+
+		if (adapter->btd_adapter == device_get_adapter(device))
+			return adapter;
+	}
+
+	return NULL;
+}
+
 static void clear_configuration(struct media_endpoint *endpoint,
 					struct media_transport *transport)
 {
@@ -426,6 +440,33 @@ struct a2dp_config_data {
 	a2dp_endpoint_config_t cb;
 };
 
+int8_t media_player_get_device_volume(struct btd_device *device)
+{
+	struct avrcp_player *target_player;
+	struct media_adapter *adapter;
+	GSList *l;
+
+	if (!device)
+		return -1;
+
+	target_player = avrcp_get_target_player_by_device(device);
+	if (!target_player)
+		return -1;
+
+	adapter = find_adapter(device);
+	if (!adapter)
+		return -1;
+
+	for (l = adapter->players; l; l = l->next) {
+		struct media_player *mp = l->data;
+
+		if (mp->player == target_player)
+			return mp->volume;
+	}
+
+	return -1;
+}
+
 static gboolean set_configuration(struct media_endpoint *endpoint,
 					uint8_t *configuration, size_t size,
 					media_endpoint_cb_t cb,
@@ -439,6 +480,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 	const char *path;
 	DBusMessageIter iter;
 	struct media_transport *transport;
+	int8_t init_volume;
 
 	transport = find_device_transport(endpoint, device);
 
@@ -451,6 +493,9 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 	if (transport == NULL)
 		return FALSE;
 
+	init_volume = media_player_get_device_volume(device);
+	media_transport_update_volume(transport, init_volume);
+
 	msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
 						MEDIA_ENDPOINT_INTERFACE,
 						"SetConfiguration");
@@ -646,20 +691,6 @@ static gboolean endpoint_init_a2dp_sink(struct media_endpoint *endpoint,
 	return TRUE;
 }
 
-static struct media_adapter *find_adapter(struct btd_device *device)
-{
-	GSList *l;
-
-	for (l = adapters; l; l = l->next) {
-		struct media_adapter *adapter = l->data;
-
-		if (adapter->btd_adapter == device_get_adapter(device))
-			return adapter;
-	}
-
-	return NULL;
-}
-
 static bool endpoint_properties_exists(const char *uuid,
 						struct btd_device *dev,
 						void *user_data)
@@ -1779,6 +1810,7 @@ static struct media_player *media_player_create(struct media_adapter *adapter,
 	mp->sender = g_strdup(sender);
 	mp->path = g_strdup(path);
 	mp->timer = g_timer_new();
+	mp->volume = -1;
 
 	mp->watch = g_dbus_add_disconnect_watch(conn, sender,
 						media_player_exit, mp,
diff --git a/profiles/audio/media.h b/profiles/audio/media.h
index dd630d432..53694f4c6 100644
--- a/profiles/audio/media.h
+++ b/profiles/audio/media.h
@@ -33,3 +33,5 @@ void media_unregister(struct btd_adapter *btd_adapter);
 struct a2dp_sep *media_endpoint_get_sep(struct media_endpoint *endpoint);
 const char *media_endpoint_get_uuid(struct media_endpoint *endpoint);
 uint8_t media_endpoint_get_codec(struct media_endpoint *endpoint);
+
+int8_t media_player_get_device_volume(struct btd_device *device);
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 2ae5118c4..a2c4f7dfb 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -977,7 +977,7 @@ void media_transport_update_device_volume(struct btd_device *dev,
 {
 	GSList *l;
 
-	if (dev == NULL)
+	if (dev == NULL || volume < 0)
 		return;
 
 	for (l = transports; l; l = l->next) {
-- 
2.28.0.rc0.105.gf9edc3c819-goog


  reply	other threads:[~2020-07-23  6:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23  6:22 [Bluez PATCH v4 1/2] audio/transport: change volume to 8bit Archie Pusaka
2020-07-23  6:22 ` Archie Pusaka [this message]
2020-07-23  6:49 ` [Bluez,v4,1/2] " bluez.test.bot
2020-07-23  6:51 ` bluez.test.bot

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=20200723142128.Bluez.v4.2.Ic16589fde45fac0c496dfca2fa27672059114c3b@changeid \
    --to=apusaka@google.com \
    --cc=apusaka@chromium.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=yudiliu@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).