2020-07-09 16:08:00

by Yu Liu

[permalink] [raw]
Subject: [Bluez PATCH v1] audio/media - Fix volume sync between media and transport

From: Hsin-Yu Chao <[email protected]>

A volume value is cached on the global media player object. And a
check was used to NOT update volume to each transport if this
value doesn't change. That is causing problem at disconnect then
reconnect when the new constructed transport never receive update
about the last used volume value.

Reviewed-by: [email protected]
Reviewed-by: [email protected]

---

Changes in v1:
- Initial change

profiles/audio/media.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 993ecb3b3..92e363de9 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -1204,9 +1204,6 @@ static void set_volume(uint8_t volume, struct btd_device *dev, void *user_data)
struct media_player *mp = user_data;
GSList *l;

- if (mp->volume == volume)
- return;
-
mp->volume = volume;

for (l = mp->adapter->endpoints; l; l = l->next) {
--
2.27.0.383.g050319c2ae-goog


2020-07-10 20:35:54

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [Bluez PATCH v1] audio/media - Fix volume sync between media and transport

Hi,

On Thu, Jul 9, 2020 at 9:10 AM Yu Liu <[email protected]> wrote:
>
> From: Hsin-Yu Chao <[email protected]>
>
> A volume value is cached on the global media player object. And a
> check was used to NOT update volume to each transport if this
> value doesn't change. That is causing problem at disconnect then
> reconnect when the new constructed transport never receive update
> about the last used volume value.

I think this might be related to the other bug we have where the
transport is created after the fetch of the volume so the volume we
have stored in mp->volume is never updated in the transport, see my
comments on the other patch.

> Reviewed-by: [email protected]
> Reviewed-by: [email protected]
>
> ---
>
> Changes in v1:
> - Initial change
>
> profiles/audio/media.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index 993ecb3b3..92e363de9 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -1204,9 +1204,6 @@ static void set_volume(uint8_t volume, struct btd_device *dev, void *user_data)
> struct media_player *mp = user_data;
> GSList *l;
>
> - if (mp->volume == volume)
> - return;
> -
> mp->volume = volume;
>
> for (l = mp->adapter->endpoints; l; l = l->next) {
> --
> 2.27.0.383.g050319c2ae-goog
>


--
Luiz Augusto von Dentz

2020-07-10 21:00:09

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [Bluez PATCH v1] audio/media - Fix volume sync between media and transport

On Fri, Jul 10, 2020 at 1:48 PM Yu Liu <[email protected]> wrote:
>
> Sorry, which patch?

From Archie, subject: [Bluez PATCH v1 2/2] audio/transport: store
volume for initialization

> On Fri, Jul 10, 2020 at 1:35 PM Luiz Augusto von Dentz <[email protected]> wrote:
>>
>> Hi,
>>
>> On Thu, Jul 9, 2020 at 9:10 AM Yu Liu <[email protected]> wrote:
>> >
>> > From: Hsin-Yu Chao <[email protected]>
>> >
>> > A volume value is cached on the global media player object. And a
>> > check was used to NOT update volume to each transport if this
>> > value doesn't change. That is causing problem at disconnect then
>> > reconnect when the new constructed transport never receive update
>> > about the last used volume value.
>>
>> I think this might be related to the other bug we have where the
>> transport is created after the fetch of the volume so the volume we
>> have stored in mp->volume is never updated in the transport, see my
>> comments on the other patch.
>>
>> > Reviewed-by: [email protected]
>> > Reviewed-by: [email protected]
>> >
>> > ---
>> >
>> > Changes in v1:
>> > - Initial change
>> >
>> > profiles/audio/media.c | 3 ---
>> > 1 file changed, 3 deletions(-)
>> >
>> > diff --git a/profiles/audio/media.c b/profiles/audio/media.c
>> > index 993ecb3b3..92e363de9 100644
>> > --- a/profiles/audio/media.c
>> > +++ b/profiles/audio/media.c
>> > @@ -1204,9 +1204,6 @@ static void set_volume(uint8_t volume, struct btd_device *dev, void *user_data)
>> > struct media_player *mp = user_data;
>> > GSList *l;
>> >
>> > - if (mp->volume == volume)
>> > - return;
>> > -
>> > mp->volume = volume;
>> >
>> > for (l = mp->adapter->endpoints; l; l = l->next) {
>> > --
>> > 2.27.0.383.g050319c2ae-goog
>> >
>>
>>
>> --
>> Luiz Augusto von Dentz



--
Luiz Augusto von Dentz