[PATCH bluez v4 0/2] audio: Don't initialize media transport volume from media player

[PATCH bluez v4 0/2] audio: Don't initialize media transport volume from media player

[Myrrh Periwinkle]

Fixes an issue where if a Bluetooth headphone with hardware volume
control was connected while mpris-proxy was running, disconnected and
replaced with a different one without hardware volume control, the
latter will be erroneously recognized as hardware volume
control capable.

---
Changes in v4:
- Remove unused variables
- Link to v3: https://patch.msgid.link/20250806-audio-no-reuse-media-player-volume-v3-0-a64a2e22f719@qtmlabs.xyz

Changes in v3:
- Actually remove the correct field
- Link to v2: https://patch.msgid.link/20250806-audio-no-reuse-media-player-volume-v2-0-cfeafb7dee0c@qtmlabs.xyz

Changes in v2:
- Remove media_player.volume
- Link to v1: https://patch.msgid.link/20250805-audio-no-reuse-media-player-volume-v1-1-c9fdfaf74a71@qtmlabs.xyz

---
Myrrh Periwinkle (2):
      audio: Don't initialize media transport volume from media player
      audio: Remove media_player.volume

 profiles/audio/avrcp.c | 10 +---------
 profiles/audio/avrcp.h |  2 --
 profiles/audio/media.c | 47 +----------------------------------------------
 profiles/audio/media.h |  1 -
 4 files changed, 2 insertions(+), 58 deletions(-)
---
base-commit: 2c0c323d08357a4ff3065fcd49fee0c83b5835cd
change-id: 20250805-audio-no-reuse-media-player-volume-fbc2983a287a

Best regards,
-- 
Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>

[PATCH bluez v4 1/2] audio: Don't initialize media transport volume from media player

[Myrrh Periwinkle]

Media player objects may be shared between devices. As a result,
a device without support for hardware volume that is connected after one
that does may end up being erroneously considered hardware
volume-capable.

fa7828bdd ("transport: Fix not being able to initialize volume properly")
introduced btd_device_{get,set}_volume that is used as an alternative in
case no media player objects are present. Therefore, we can remove
media_player_get_device_volume and instead use btd_device_get_volume to
determine the initial volume.
---
 profiles/audio/avrcp.c |  2 +-
 profiles/audio/media.c | 33 +--------------------------------
 profiles/audio/media.h |  1 -
 3 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index e2797112fcd580c3fc56793f933e00b1c61e5205..ec07522e6a34eb1dc5f6f413f48f1087a609df9a 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -4284,7 +4284,7 @@ static void target_init(struct avrcp *session)
 		target->player = player;
 		player->sessions = g_slist_prepend(player->sessions, session);
 
-		init_volume = media_player_get_device_volume(session->dev);
+		init_volume = btd_device_get_volume(session->dev);
 		media_transport_update_device_volume(session->dev, init_volume);
 	}
 
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 8e62dca17070edbc5101677c6eebd3707492c824..55f1482d1d9ce52e104481bab3ede373f47aee0c 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -499,37 +499,6 @@ struct a2dp_config_data {
 	a2dp_endpoint_config_t cb;
 };
 
-int8_t media_player_get_device_volume(struct btd_device *device)
-{
-#ifdef HAVE_AVRCP
-	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)
-		goto done;
-
-	adapter = find_adapter(device);
-	if (!adapter)
-		goto done;
-
-	for (l = adapter->players; l; l = l->next) {
-		struct media_player *mp = l->data;
-
-		if (mp->player == target_player)
-			return mp->volume;
-	}
-
-done:
-#endif /* HAVE_AVRCP */
-	/* If media_player doesn't exists use device_volume */
-	return btd_device_get_volume(device);
-}
-
 static gboolean set_configuration(struct media_endpoint *endpoint,
 					uint8_t *configuration, size_t size,
 					media_endpoint_cb_t cb,
@@ -556,7 +525,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 	if (transport == NULL)
 		return FALSE;
 
-	init_volume = media_player_get_device_volume(device);
+	init_volume = btd_device_get_volume(device);
 	media_transport_update_volume(transport, init_volume);
 
 	msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
diff --git a/profiles/audio/media.h b/profiles/audio/media.h
index 2b2e8e1572874d5f71abb28fdd5b92fa2d9efe83..d3954abd6de505a69cab3fcffc217d236a52d3e5 100644
--- a/profiles/audio/media.h
+++ b/profiles/audio/media.h
@@ -23,6 +23,5 @@ uint8_t media_endpoint_get_codec(struct media_endpoint *endpoint);
 struct btd_adapter *media_endpoint_get_btd_adapter(
 					struct media_endpoint *endpoint);
 bool media_endpoint_is_broadcast(struct media_endpoint *endpoint);
-int8_t media_player_get_device_volume(struct btd_device *device);
 
 const struct media_endpoint *media_endpoint_get_asha(void);

-- 
2.50.1

[PATCH bluez v4 2/2] audio: Remove media_player.volume

[Myrrh Periwinkle]

With the previous commit, the media_player.volume field is now
completely unused. As this was never the right place to store the
initial volume to begin with, remove this field entirely as well as its
associated supporting methods to prevent confusion.
---
 profiles/audio/avrcp.c |  8 --------
 profiles/audio/avrcp.h |  2 --
 profiles/audio/media.c | 14 --------------
 3 files changed, 24 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index ec07522e6a34eb1dc5f6f413f48f1087a609df9a..0cb2ad657534d0c02a51e095ac76f3e3f28376af 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -3796,7 +3796,6 @@ static void avrcp_get_media_player_list(struct avrcp *session)
 static void avrcp_volume_changed(struct avrcp *session,
 						struct avrcp_header *pdu)
 {
-	struct avrcp_player *player = target_get_player(session);
 	int8_t volume;
 
 	if (!avrcp_volume_supported(session->controller)) {
@@ -3808,9 +3807,6 @@ static void avrcp_volume_changed(struct avrcp *session,
 
 	/* Always attempt to update the transport volume */
 	media_transport_update_device_volume(session->dev, volume);
-
-	if (player)
-		player->cb->set_volume(volume, session->dev, player->user_data);
 }
 
 static void avrcp_status_changed(struct avrcp *session,
@@ -4637,7 +4633,6 @@ static gboolean avrcp_handle_set_volume(struct avctp *conn, uint8_t code,
 					void *user_data)
 {
 	struct avrcp *session = user_data;
-	struct avrcp_player *player = target_get_player(session);
 	struct avrcp_header *pdu = (void *) operands;
 	int8_t volume;
 
@@ -4650,9 +4645,6 @@ 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 (player != NULL)
-		player->cb->set_volume(volume, session->dev, player->user_data);
-
 	return FALSE;
 }
 
diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
index 887753ddf28643800fabbddb1bf0941ed3930463..1f46df17d48e7c1d22ccf97bc7804e21372f5882 100644
--- a/profiles/audio/avrcp.h
+++ b/profiles/audio/avrcp.h
@@ -83,8 +83,6 @@ struct avrcp_player_cb {
 	uint32_t (*get_position)(void *user_data);
 	uint32_t (*get_duration)(void *user_data);
 	const char *(*get_name)(void *user_data);
-	void (*set_volume)(int8_t volume, struct btd_device *dev,
-							void *user_data);
 	bool (*play)(void *user_data);
 	bool (*stop)(void *user_data);
 	bool (*pause)(void *user_data);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 55f1482d1d9ce52e104481bab3ede373f47aee0c..fd6e30f042ecee9882694059fcf72daf5e0eb436 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -142,7 +142,6 @@ struct media_player {
 	char			*status;
 	uint32_t		position;
 	uint32_t		duration;
-	int8_t			volume;
 	GTimer			*timer;
 	bool			play;
 	bool			pause;
@@ -2036,17 +2035,6 @@ static uint32_t media_player_get_duration(void *user_data)
 	return mp->duration;
 }
 
-static void media_player_set_volume(int8_t volume, struct btd_device *dev,
-				    void *user_data)
-{
-	struct media_player *mp = user_data;
-
-	if (mp->volume == volume)
-		return;
-
-	mp->volume = volume;
-}
-
 static bool media_player_send(struct media_player *mp, const char *name)
 {
 	DBusMessage *msg;
@@ -2134,7 +2122,6 @@ static struct avrcp_player_cb player_cb = {
 	.get_duration = media_player_get_duration,
 	.get_status = media_player_get_status,
 	.get_name = media_player_get_player_name,
-	.set_volume = media_player_set_volume,
 	.play = media_player_play,
 	.stop = media_player_stop,
 	.pause = media_player_pause,
@@ -2617,7 +2604,6 @@ 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,

-- 
2.50.1

RE: audio: Don't initialize media transport volume from media player

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=988599

---Test result---

Test Summary:
CheckPatch                    PENDING   0.27 seconds
GitLint                       PENDING   0.20 seconds
BuildEll                      PASS      20.13 seconds
BluezMake                     PASS      2721.55 seconds
MakeCheck                     PASS      19.95 seconds
MakeDistcheck                 PASS      184.17 seconds
CheckValgrind                 PASS      233.69 seconds
CheckSmatch                   PASS      306.56 seconds
bluezmakeextell               PASS      127.79 seconds
IncrementalBuild              PENDING   0.29 seconds
ScanBuild                     PASS      919.18 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH bluez v4 0/2] audio: Don't initialize media transport volume from media player

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 06 Aug 2025 09:22:45 +0700 you wrote:
> Fixes an issue where if a Bluetooth headphone with hardware volume
> control was connected while mpris-proxy was running, disconnected and
> replaced with a different one without hardware volume control, the
> latter will be erroneously recognized as hardware volume
> control capable.
> 
> 
> [...]

Here is the summary with links:
  - [bluez,v4,1/2] audio: Don't initialize media transport volume from media player
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5db6d2fef66f
  - [bluez,v4,2/2] audio: Remove media_player.volume
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9cdbad590b74

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html