diff mbox series

[Bluez,v1] audio/media - Fix volume sync between media and transport

Message ID 20200709090631.Bluez.v1.1.I6aa22c6e425e5b19c321c0768f50ca3fc2c090da@changeid (mailing list archive)
State New, archived
Headers show
Series [Bluez,v1] audio/media - Fix volume sync between media and transport | expand

Commit Message

Yu Liu July 9, 2020, 4:06 p.m. UTC
From: Hsin-Yu Chao <hychao@chromium.org>

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: sonnysasaka@chromium.org
Reviewed-by: hychao@chromium.org

---

Changes in v1:
- Initial change

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

Comments

Luiz Augusto von Dentz July 10, 2020, 8:35 p.m. UTC | #1
Hi,

On Thu, Jul 9, 2020 at 9:10 AM Yu Liu <yudiliu@google.com> wrote:
>
> From: Hsin-Yu Chao <hychao@chromium.org>
>
> 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: sonnysasaka@chromium.org
> Reviewed-by: hychao@chromium.org
>
> ---
>
> 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 July 10, 2020, 8:58 p.m. UTC | #2
On Fri, Jul 10, 2020 at 1:48 PM Yu Liu <yudiliu@google.com> 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 <luiz.dentz@gmail.com> wrote:
>>
>> Hi,
>>
>> On Thu, Jul 9, 2020 at 9:10 AM Yu Liu <yudiliu@google.com> wrote:
>> >
>> > From: Hsin-Yu Chao <hychao@chromium.org>
>> >
>> > 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: sonnysasaka@chromium.org
>> > Reviewed-by: hychao@chromium.org
>> >
>> > ---
>> >
>> > 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
diff mbox series

Patch

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) {