| Message ID | 20200831154443.Bluez.v1.1.Ieeae14ab680eda03474551fdb7a0a020f950e9c1@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [Bluez,v1] media: Don't set initial volume if it's invalid | expand |
Hi Archie, On Mon, Aug 31, 2020 at 12:45 AM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > When initializing media transport, we try to initialize the volume > of the player. However, the assigned initial volume could be invalid > due to the session has not been initialized, or when we assume the > role of audio sink. In this case, we should not assign the initial > volume. Not really following the explanation here, if the session has not been initialized yet shouldn't the volume be actually invalid? Or is the problem that we don't call media_transport_update_volume later when it is initialized? > Reviewed-by: Michael Sun <michaelfsun@google.com> > Reviewed-by: Yu Liu <yudiliu@google.com> > --- > > profiles/audio/media.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/profiles/audio/media.c b/profiles/audio/media.c > index 02bf82a49..acb4a8ee9 100644 > --- a/profiles/audio/media.c > +++ b/profiles/audio/media.c > @@ -494,7 +494,8 @@ static gboolean set_configuration(struct media_endpoint *endpoint, > return FALSE; > > init_volume = media_player_get_device_volume(device); > - media_transport_update_volume(transport, init_volume); > + if (init_volume >= 0) > + media_transport_update_volume(transport, init_volume); You can probably move the check to be done internally inside update_volume. > msg = dbus_message_new_method_call(endpoint->sender, endpoint->path, > MEDIA_ENDPOINT_INTERFACE, > -- > 2.28.0.402.g5ffc5be6b7-goog >
Hi Luiz, On Tue, 1 Sep 2020 at 01:20, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Archie, > > On Mon, Aug 31, 2020 at 12:45 AM Archie Pusaka <apusaka@google.com> wrote: > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > When initializing media transport, we try to initialize the volume > > of the player. However, the assigned initial volume could be invalid > > due to the session has not been initialized, or when we assume the > > role of audio sink. In this case, we should not assign the initial > > volume. > > Not really following the explanation here, if the session has not been > initialized yet shouldn't the volume be actually invalid? Or is the > problem that we don't call media_transport_update_volume later when it > is initialized? Yes, the volume should be invalid by that time. However, the default value is invalid anyway so there is no need to reassign an invalid volume again. We still call media_transport_update_volume when the session is initialized later. > > > Reviewed-by: Michael Sun <michaelfsun@google.com> > > Reviewed-by: Yu Liu <yudiliu@google.com> > > --- > > > > profiles/audio/media.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/profiles/audio/media.c b/profiles/audio/media.c > > index 02bf82a49..acb4a8ee9 100644 > > --- a/profiles/audio/media.c > > +++ b/profiles/audio/media.c > > @@ -494,7 +494,8 @@ static gboolean set_configuration(struct media_endpoint *endpoint, > > return FALSE; > > > > init_volume = media_player_get_device_volume(device); > > - media_transport_update_volume(transport, init_volume); > > + if (init_volume >= 0) > > + media_transport_update_volume(transport, init_volume); > > You can probably move the check to be done internally inside update_volume. Correct, I will do so. > > > msg = dbus_message_new_method_call(endpoint->sender, endpoint->path, > > MEDIA_ENDPOINT_INTERFACE, > > -- > > 2.28.0.402.g5ffc5be6b7-goog > > > > > -- > Luiz Augusto von Dentz Thanks, Archie
diff --git a/profiles/audio/media.c b/profiles/audio/media.c index 02bf82a49..acb4a8ee9 100644 --- a/profiles/audio/media.c +++ b/profiles/audio/media.c @@ -494,7 +494,8 @@ static gboolean set_configuration(struct media_endpoint *endpoint, return FALSE; init_volume = media_player_get_device_volume(device); - media_transport_update_volume(transport, init_volume); + if (init_volume >= 0) + media_transport_update_volume(transport, init_volume); msg = dbus_message_new_method_call(endpoint->sender, endpoint->path, MEDIA_ENDPOINT_INTERFACE,