Re: [pulseaudio-discuss] [RFC 2/2] bluetooth: Rank profiles based on available flag
Hi Tanu,
On Mon, Nov 18, 2013 at 9:36 AM, Tanu Kaskinen
tanu.kaski...@linux.intel.com wrote:
On Tue, 2013-11-12 at 13:02 +0200, Luiz Augusto von Dentz wrote:
From: Luiz Augusto von Dentz luiz.von.de...@intel.com
This makes module-bluetooth-policy to rank profiles based on their
available flag and only consider their priority in case profiles have
the same rank.
The old code already ranks profiles based on their available flag and
only considers their priority in case profiles have the same rank, so
the commit message doesn't really justify the patch.
Yep, in a very odd fashion that is probably why I thought they didn't, still...
The only behaviour change (apart from the probably unintentional change
of preferring no over unknown) that I can see is that the off
profile is now preferred over profiles that have their availability set
to unknown, because the off profile has availability yes. If that
is intended, please explain why you are doing that change.
Profile with higher availability should be considered first in the
following order: yes unknown no, or perhaps Im missing something.
This means whenever a Bluetooth profile is disconnected, yes -
unknown, it should switch to 'off' if there are no profile with
available set to yes.
---
src/modules/bluetooth/module-bluetooth-policy.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-policy.c
b/src/modules/bluetooth/module-bluetooth-policy.c
index 4a90db7..864d10d 100644
--- a/src/modules/bluetooth/module-bluetooth-policy.c
+++ b/src/modules/bluetooth/module-bluetooth-policy.c
@@ -137,21 +137,22 @@ static pa_card_profile *find_best_profile(pa_card
*card) {
void *state;
pa_card_profile *profile;
pa_card_profile *result = card-active_profile;
-pa_card_profile *off;
-
-pa_assert_se(off = pa_hashmap_get(card-profiles, off));
PA_HASHMAP_FOREACH(profile, card-profiles, state) {
-if (profile-available == PA_AVAILABLE_NO || profile == off)
+if (result == profile)
+continue;
+
+if (profile-available result-available) {
Unknown is defined as 0 and no is defined as 1, so no will be
preferred over unknown, which is probably not what you intended.
Now I figure why the checks for available was done in such way, still
there is no point in disregard 'off' profile like the was doing as in
99% of the time it is the best/most available profile in the
circumstances where find_best_profile is called.
--
Luiz Augusto von Dentz
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC 2/2] bluetooth: Rank profiles based on available flag
On Mon, 2013-11-18 at 16:20 +0200, Luiz Augusto von Dentz wrote:
Hi Tanu,
On Mon, Nov 18, 2013 at 9:36 AM, Tanu Kaskinen
tanu.kaski...@linux.intel.com wrote:
On Tue, 2013-11-12 at 13:02 +0200, Luiz Augusto von Dentz wrote:
diff --git a/src/modules/bluetooth/module-bluetooth-policy.c
b/src/modules/bluetooth/module-bluetooth-policy.c
index 4a90db7..864d10d 100644
--- a/src/modules/bluetooth/module-bluetooth-policy.c
+++ b/src/modules/bluetooth/module-bluetooth-policy.c
@@ -137,21 +137,22 @@ static pa_card_profile *find_best_profile(pa_card
*card) {
void *state;
pa_card_profile *profile;
pa_card_profile *result = card-active_profile;
-pa_card_profile *off;
-
-pa_assert_se(off = pa_hashmap_get(card-profiles, off));
PA_HASHMAP_FOREACH(profile, card-profiles, state) {
-if (profile-available == PA_AVAILABLE_NO || profile == off)
+if (result == profile)
+continue;
+
+if (profile-available result-available) {
Unknown is defined as 0 and no is defined as 1, so no will be
preferred over unknown, which is probably not what you intended.
Now I figure why the checks for available was done in such way, still
there is no point in disregard 'off' profile like the was doing as in
99% of the time it is the best/most available profile in the
circumstances where find_best_profile is called.
I think there is a point in prioritizing off below other profiles
whose state is unknown. There are three transport states: disconnected,
connected and playing. You are saying that when the transport state
changes from playing to connected, then we should change the card
profile to off. I think this is a very bad idea. Let's say you're
playing music to a BT sink. Then you pause the music, the stream gets
corked, the sink gets suspended. When the sink suspends, the transport
is released and the transport state changes from playing to connected.
In your proposal, pausing the music player changes the BT card profile
to off, forcing rerouting (or killing) of the music stream.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC 2/2] bluetooth: Rank profiles based on available flag
Hi Tanu,
On Mon, Nov 18, 2013 at 4:32 PM, Tanu Kaskinen
tanu.kaski...@linux.intel.com wrote:
On Mon, 2013-11-18 at 16:20 +0200, Luiz Augusto von Dentz wrote:
Hi Tanu,
On Mon, Nov 18, 2013 at 9:36 AM, Tanu Kaskinen
tanu.kaski...@linux.intel.com wrote:
On Tue, 2013-11-12 at 13:02 +0200, Luiz Augusto von Dentz wrote:
diff --git a/src/modules/bluetooth/module-bluetooth-policy.c
b/src/modules/bluetooth/module-bluetooth-policy.c
index 4a90db7..864d10d 100644
--- a/src/modules/bluetooth/module-bluetooth-policy.c
+++ b/src/modules/bluetooth/module-bluetooth-policy.c
@@ -137,21 +137,22 @@ static pa_card_profile *find_best_profile(pa_card
*card) {
void *state;
pa_card_profile *profile;
pa_card_profile *result = card-active_profile;
-pa_card_profile *off;
-
-pa_assert_se(off = pa_hashmap_get(card-profiles, off));
PA_HASHMAP_FOREACH(profile, card-profiles, state) {
-if (profile-available == PA_AVAILABLE_NO || profile == off)
+if (result == profile)
+continue;
+
+if (profile-available result-available) {
Unknown is defined as 0 and no is defined as 1, so no will be
preferred over unknown, which is probably not what you intended.
Now I figure why the checks for available was done in such way, still
there is no point in disregard 'off' profile like the was doing as in
99% of the time it is the best/most available profile in the
circumstances where find_best_profile is called.
I think there is a point in prioritizing off below other profiles
whose state is unknown. There are three transport states: disconnected,
connected and playing. You are saying that when the transport state
changes from playing to connected, then we should change the card
profile to off. I think this is a very bad idea. Let's say you're
playing music to a BT sink. Then you pause the music, the stream gets
corked, the sink gets suspended. When the sink suspends, the transport
is released and the transport state changes from playing to connected.
In your proposal, pausing the music player changes the BT card profile
to off, forcing rerouting (or killing) of the music stream.
Except that this policy is not used in this case, source/phone/desktop
profiles are not being handled by module-bluetooth-policy. Btw for
those profiles they should probably be considered available 'yes'
always if transport state is different than disconnected and they
normally are switched directly by the user application such as a media
player that should request A2DP/music stream or dialer/skype that
request HFP/voice stream.
--
Luiz Augusto von Dentz
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC 2/2] bluetooth: Rank profiles based on available flag
On Mon, 2013-11-18 at 17:06 +0200, Luiz Augusto von Dentz wrote:
Hi Tanu,
On Mon, Nov 18, 2013 at 4:32 PM, Tanu Kaskinen
tanu.kaski...@linux.intel.com wrote:
On Mon, 2013-11-18 at 16:20 +0200, Luiz Augusto von Dentz wrote:
Hi Tanu,
On Mon, Nov 18, 2013 at 9:36 AM, Tanu Kaskinen
tanu.kaski...@linux.intel.com wrote:
On Tue, 2013-11-12 at 13:02 +0200, Luiz Augusto von Dentz wrote:
diff --git a/src/modules/bluetooth/module-bluetooth-policy.c
b/src/modules/bluetooth/module-bluetooth-policy.c
index 4a90db7..864d10d 100644
--- a/src/modules/bluetooth/module-bluetooth-policy.c
+++ b/src/modules/bluetooth/module-bluetooth-policy.c
@@ -137,21 +137,22 @@ static pa_card_profile *find_best_profile(pa_card
*card) {
void *state;
pa_card_profile *profile;
pa_card_profile *result = card-active_profile;
-pa_card_profile *off;
-
-pa_assert_se(off = pa_hashmap_get(card-profiles, off));
PA_HASHMAP_FOREACH(profile, card-profiles, state) {
-if (profile-available == PA_AVAILABLE_NO || profile == off)
+if (result == profile)
+continue;
+
+if (profile-available result-available) {
Unknown is defined as 0 and no is defined as 1, so no will be
preferred over unknown, which is probably not what you intended.
Now I figure why the checks for available was done in such way, still
there is no point in disregard 'off' profile like the was doing as in
99% of the time it is the best/most available profile in the
circumstances where find_best_profile is called.
I think there is a point in prioritizing off below other profiles
whose state is unknown. There are three transport states: disconnected,
connected and playing. You are saying that when the transport state
changes from playing to connected, then we should change the card
profile to off. I think this is a very bad idea. Let's say you're
playing music to a BT sink. Then you pause the music, the stream gets
corked, the sink gets suspended. When the sink suspends, the transport
is released and the transport state changes from playing to connected.
In your proposal, pausing the music player changes the BT card profile
to off, forcing rerouting (or killing) of the music stream.
Except that this policy is not used in this case, source/phone/desktop
profiles are not being handled by module-bluetooth-policy. Btw for
those profiles they should probably be considered available 'yes'
always if transport state is different than disconnected and they
normally are switched directly by the user application such as a media
player that should request A2DP/music stream or dialer/skype that
request HFP/voice stream.
I don't really care whether a profile availability is set to yes or
unknown (I think the distinction doesn't make much sense, it exists
only because someone, probably you or Mikel, didn't want to access the
transport states directly from module-bluetooth-policy). So feel free to
do any modifications in that space if you wish (as long as it doesn't
break anything, of course).
I didn't remember that the code that this patch modifies doesn't affect
PC use cases. If you want to switch to the off profile when the HFGW
transport state changes from playing to connected, I'm fine with that.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [RFC 2/2] bluetooth: Rank profiles based on available flag
On Tue, 2013-11-12 at 13:02 +0200, Luiz Augusto von Dentz wrote:
From: Luiz Augusto von Dentz luiz.von.de...@intel.com
This makes module-bluetooth-policy to rank profiles based on their
available flag and only consider their priority in case profiles have
the same rank.
The old code already ranks profiles based on their available flag and
only considers their priority in case profiles have the same rank, so
the commit message doesn't really justify the patch.
The only behaviour change (apart from the probably unintentional change
of preferring no over unknown) that I can see is that the off
profile is now preferred over profiles that have their availability set
to unknown, because the off profile has availability yes. If that
is intended, please explain why you are doing that change.
---
src/modules/bluetooth/module-bluetooth-policy.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-policy.c
b/src/modules/bluetooth/module-bluetooth-policy.c
index 4a90db7..864d10d 100644
--- a/src/modules/bluetooth/module-bluetooth-policy.c
+++ b/src/modules/bluetooth/module-bluetooth-policy.c
@@ -137,21 +137,22 @@ static pa_card_profile *find_best_profile(pa_card
*card) {
void *state;
pa_card_profile *profile;
pa_card_profile *result = card-active_profile;
-pa_card_profile *off;
-
-pa_assert_se(off = pa_hashmap_get(card-profiles, off));
PA_HASHMAP_FOREACH(profile, card-profiles, state) {
-if (profile-available == PA_AVAILABLE_NO || profile == off)
+if (result == profile)
+continue;
+
+if (profile-available result-available) {
Unknown is defined as 0 and no is defined as 1, so no will be
preferred over unknown, which is probably not what you intended.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [RFC 2/2] bluetooth: Rank profiles based on available flag
From: Luiz Augusto von Dentz luiz.von.de...@intel.com
This makes module-bluetooth-policy to rank profiles based on their
available flag and only consider their priority in case profiles have
the same rank.
---
src/modules/bluetooth/module-bluetooth-policy.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/modules/bluetooth/module-bluetooth-policy.c
b/src/modules/bluetooth/module-bluetooth-policy.c
index 4a90db7..864d10d 100644
--- a/src/modules/bluetooth/module-bluetooth-policy.c
+++ b/src/modules/bluetooth/module-bluetooth-policy.c
@@ -137,21 +137,22 @@ static pa_card_profile *find_best_profile(pa_card *card) {
void *state;
pa_card_profile *profile;
pa_card_profile *result = card-active_profile;
-pa_card_profile *off;
-
-pa_assert_se(off = pa_hashmap_get(card-profiles, off));
PA_HASHMAP_FOREACH(profile, card-profiles, state) {
-if (profile-available == PA_AVAILABLE_NO || profile == off)
+if (result == profile)
+continue;
+
+if (profile-available result-available) {
+result = profile;
continue;
+}
-if (result == NULL ||
-(profile-available == PA_AVAILABLE_YES result-available ==
PA_AVAILABLE_UNKNOWN) ||
-(profile-available == result-available profile-priority
profile-priority))
+if (profile-available == result-available
+profile-priority profile-priority)
result = profile;
}
-return result ? result : off;
+return result;
}
static pa_hook_result_t profile_available_hook_callback(pa_core *c,
pa_card_profile *profile, void *userdata) {
--
1.8.3.1
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
