Re: [pulseaudio-discuss] [RFC 2/2] bluetooth: Rank profiles based on available flag

[Tanu Kaskinen]

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
>  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
> >>  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] card: Only set active_profile with available profile

[Tanu Kaskinen]

On Mon, 2013-11-18 at 16:51 +0200, Luiz Augusto von Dentz wrote:
> Hi Tanu,
> 
> On Mon, Nov 18, 2013 at 9:25 AM, Tanu Kaskinen
>  wrote:
> > On Sun, 2013-11-17 at 17:53 +0200, Luiz Augusto von Dentz wrote:
> >> Hi Colin, Tanu,
> >>
> >> On Sun, Nov 17, 2013 at 3:17 PM, Colin Guthrie  
> >> wrote:
> >> > 'Twas brillig, and Tanu Kaskinen at 16/11/13 08:51 did gyre and gimble:
> >> >> On Fri, 2013-11-15 at 09:31 +0100, Colin Guthrie wrote:
> >> >> I haven't concentrated on the patches enough to even know what
> >> >> problem you're solving - is it just that the assertion makes you
> >> >> nervous, or is there some other benefit.
> >> >
> >> > I'm not solving any specific problem per-se. I'm just looking at several
> >> > bugs where incorrect profiles appear to be selected by default on first
> >> > boot where HDMI is picked over regular analog output (for those alsa
> >> > cards which share a single card for both HDMI and Analog output rather
> >> > than having it presented as separate cards) despite HDMI having a lower
> >> > priority. This lead me to look at this code and got me thinking - this
> >> > is the only reason for the patch.
> >> >
> >> > A further complication to my current bugs is that sometimes Headphone
> >> > *ports* are picked by default on first boot even when they are
> >> > unavailable and again are not the highest priority port. It seems like
> >> > the same problem but in a different, but similar, part of the code. This
> >> > is why the second set of patches applied the same logical fixes to the
> >> > Sink/Source code that Luiz added to the Card profiles. I think it makes
> >> > sense to keep these bits of code approximately in sync regardless of the
> >> > outcome of this discussion - the same logic should apply.
> >>
> >> I can try and apply the same logic to Sink/Source, it just that
> >> overall this kind of policy should be in one place or then remove it
> >> altogether and make sure the list is always ordered by their module so
> >> the core should just pick the first one, the problem we were trying to
> >> solve was that there is no hook for priority changes thus they are
> >> considered static which is probably why we are relying on
> >> pa_card_set_profile_available +
> >> PA_CORE_HOOK_CARD_PROFILE_AVAILABLE_CHANGED to indicate the profiles
> >> changes.
> >>
> >> So perhaps we should came to an agreement where and how we should be
> >> doing this sort of policy, we could perhaps generalize
> >> module-bluetooth-policy, or do we want to continue with per technology
> >> policy?
> >
> > I don't think the policy in module-bluetooth-policy is useful outside
> > Bluetooth. module-bluetooth-policy is mainly concerned with the
> > transport state in its policy, and the profile availability is only used
> > as a proxy for the transport state. I'd be happy if
> > module-bluetooth-policy would be changed so that instead of using the
> > profile states, it would directly track the transport states using the
> > APIs that pa_bluez_discovery provides. But since using the profile
> > states as a proxy works fine too, I'm not really asking anyone to do
> > anything about that.
> >
> > From the proposed changes, I think Colin's patches 1 and 3 make sense.
> > Preferring PA_AVAILABLE_YES over PA_AVAILABLE_UNKNOWN in generic code
> > isn't a good idea.
> 
> Im having a hard time to understand why any policy would prefer a
> profile that is less likely to work than another,

If you have a "bad" (not very well suited for the use case) profile and
a "good" (very well suited for the use case) profile, and the
probability of the "bad" profile working is 100% and the probability of
the "good" profile working is 99%, do you really see no reason to choose
the "good" profile?

> but perhaps we are
> not willing the mess with other parts of the code that were using this
> information in a different way. But then why we are bothering
> selecting any profile on pa_card_new? This is a policy decision and it
> seems we are not able to do a generic one so it should not even try,
> pick the first and be done with it.

Yes, it's a policy decision. I would prefer pa_card_new() to always
choose the "off" profile by default, but that would require work in
policy modules to keep old use cases working. For example, on the first
boot of a PC something else than "off" should be selected for the sound
card, and currently we don't have any policy module that would ensure
that.

> Anyway if pa_card_new do not select 'off' by default it pretty much
> breaks Bluetooth qualification and if I change the priority of 'off'
> profile to force it to be selected it breaks module-bluetooth-policy.

Can you tell the Bluetooth qualification requirement that is failing? If
it's something HFGW specific, you should be able to implement the
desired policy in module-bluetooth-policy, without modifying
pa_card_new().

-- 
Tanu

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.free

[pulseaudio-discuss] How to set media role variable ??

[sathishkumar sivagurunathan]

Hi,

   I have a set of media streams running. For example; a vlc player and
mplayer.

   Assuming both the player dont have the media.role variable set, are
these steps the way to go ??

   1) Try to connect to the pulse audio server using a new context, say
"test"..

   2) Try to list all the clients.

   3) Select a clients, and using

+pa_proplist *proplist;
+
+proplist = pa_proplist_new ();

+pa_proplist_sets (proplist,
+  PA_PROP_MEDIA_ROLE,
+  "phone");

+pacOperation = pa_context_proplist_update (
pa_context*
*c*,

pa_update_mode_t  *mode*,

pa_proplist *  *p*,

pa_context_success_cb_t
  *cb*,

void *  *userdata*
)




  from internet,

  But in order to execute this, I need the context information for the
vlc and mplayer streams..

  My questions are

a) Is my analysis to assign media role correct ?
b) Is it possible to get the context information of another client stream ?
c) If both my previous questions/assumptions are not right, is the only way
to assign a media role is when the context for the player is created like
below


// CODE GOT FROM INTERNET.
+pa_proplist *proplist;
+
+proplist = pa_proplist_new ();
+pa_proplist_sets (proplist,
+  PA_PROP_APPLICATION_NAME,
+  "Mumble");
+pa_proplist_sets (proplist,
+  PA_PROP_APPLICATION_ID,
+  "net.sourceforge.mumble.mumble");
+pa_proplist_sets (proplist,
+  PA_PROP_APPLICATION_ICON_NAME,
+  "mumble");
+pa_proplist_sets (proplist,
+  PA_PROP_MEDIA_ROLE,
+  "phone");

+pacContext = pa_context_new_with_proplist (api, NULL, proplist);
+pa_proplist_free (proplist);

Can you please help me with this ??

Thanks,
Sathish
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Question regarding usage of pa_stream_peek() and pa_stream_drop()

[nimesh.chanchani]

Hi Tanu,

I recorded with 8000 hz mono, to reduce the file size. its uploaded.
http://www.filedropper.com/28_4 ( < 1 MB)

i tried to attach it, but it didnt work

please see other comments inline.

Regards,
Nimesh

From: Tanu Kaskinen [tanu.kaski...@linux.intel.com]
Sent: Monday, November 18, 2013 12:09 PM
To: Chanchani, Nimesh
Cc: pulseaudio-discuss@lists.freedesktop.org
Subject: Re: [pulseaudio-discuss] Question regarding usage of pa_stream_peek() 
and pa_stream_drop()

(Please don't top-post on mailing lists. See
http://en.wikipedia.org/wiki/Posting_style for an explanation about what
I mean by that.)

On Fri, 2013-11-15 at 11:10 +, nimesh.chanch...@accenture.com wrote:
> Hi Tanu,
>
> Thx for your response.
>
> to clarify:
>
> Yes, im using " pa_stream_readable_size()", to first check if there
> are readable bytes available.
> "pa_stream_peek()" doesnt have any error codes to return, it always
> returns Zero.
>
> by no data, I mean that the PCM data that gets written to the file is
> a flat line. that is "free  > 0" and data!= NULL.
> but by logs i noticed that very few bytes actually get pulled by
> pa_stream_peek() in each iteration like: 8, 6 , 59, 70,10 etc. to give
> one example. is that strange?

Yes, that's strange, if those are the actual numbers. I think the size
should always be a multiple of 4 bytes, since the sample format is 16
bits and it's a stereo stream, so the frame size is 4 bytes.

Nimesh>>These numbers were only for illustration, what i actually wanted to 
highlight was the magnitude of the number of bytes read.

>
> when I do get some data ( not a flat PCM line  but noisy)  , I get
> approx 1000 bytes pulled in each iteration by pa_stream_peek(), like
> 800,1600,1300,700 etc.
>
> i'm using threaded main loop .
>
> I am using a single context , and i'm opening 2 streams in that
> context ( one for playback and one for record) , will that create a
> problem?  I'm attaching the code for your reference.

Having two streams is no problem.

> "void PulseAudioClient::Read()" is the function to lookout for. This
> is still a test code , so please excuse the inefficiencies.

Read() doesn't lock the mainloop properly. You need to lock it whenever
you access objects outside the mainloop thread. Read() accesses the
stream object for the first time in the pa_stream_cork() call.

Nimesh>>Thanks for pointing that out. But there is a strange thing that i 
notice, after I lock the mainloop before calling pa_stream_cork().

The Flush function , doesn't receive a succede  callback , and I keep waiting 
in the while loop.
, if I move the mainloop lock to its original position in the code , i get the 
flush succede callback.

> i will upload the raw pcm file and mail the link.

Are you still planning to do that?

--
Tanu



This message is for the designated recipient only and may contain privileged, 
proprietary, or otherwise confidential information. If you have received it in 
error, please notify the sender immediately and delete the original. Any other 
use of the e-mail by you is prohibited. Where allowed by local law, electronic 
communications with Accenture and its affiliates, including e-mail and instant 
messaging (including content), may be scanned by our systems for the purposes 
of information security and assessment of internal compliance with Accenture 
policy. .
__

www.accenture.com

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] card: Only set active_profile with available profile

[Raymond Yau]

>
> > For reference the bug that covers both the afore mentioned cases is:
> > https://bugs.mageia.org/show_bug.cgi?id=11642
>
> I skimmed it through - if somebody has a "line out" port that's
> unavailable but still he has something plugged into his line out, that's
> a bug. Alsa-info and PA verbose log needed to figure out if it's PA or
> kernel.
>

 ports:
analog-output-lineout: Line Out (priority 9900, latency offset 0 usec,
available: no)
properties:

analog-output-headphones: Headphones (priority 9000, latency offset 0 usec,
available: no)
properties:
device.icon_name = "audio-headphones"
active port: 

I don't understand why headphone still remain  active port when both are
not available and lower priority than line out

Did pulseaudio switch to other sound card ?
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [PATCH] ucm: Add a FIXME comment about bad error handling

[Tanu Kaskinen]

---
 src/modules/alsa/alsa-ucm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/modules/alsa/alsa-ucm.c b/src/modules/alsa/alsa-ucm.c
index ef3ddb2..2995b32 100644
--- a/src/modules/alsa/alsa-ucm.c
+++ b/src/modules/alsa/alsa-ucm.c
@@ -1125,6 +1125,8 @@ static void alsa_mapping_add_ucm_modifier(pa_alsa_mapping 
*m, pa_alsa_ucm_modifi
 }
 
 if (channel_str) {
+/* FIXME: channel_str is unsanitized input from the UCM configuration,
+ * we should do proper error handling instead of asserting. */
 pa_assert_se(pa_atou(channel_str, &channels) == 0 && channels < 
PA_CHANNELS_MAX);
 pa_log_debug("Got channel count %" PRIu32 " for modifier", channels);
 }
-- 
1.8.3.1

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] card: Only set active_profile with available profile

[David Henningsson]

On 11/17/2013 02:17 PM, Colin Guthrie wrote:
> I'm not solving any specific problem per-se. I'm just looking at several
> bugs where incorrect profiles appear to be selected by default on first
> boot where HDMI is picked over regular analog output (for those alsa
> cards which share a single card for both HDMI and Analog output rather
> than having it presented as separate cards) despite HDMI having a lower
> priority. 

This seems strange. But a pulseaudio verbose log could probably reveal
what is happening.

Anyway, since ALSA does not use profile availability, your patch will
unlikely affect the problem, but I guess you already figured that out.

> A further complication to my current bugs is that sometimes Headphone
> *ports* are picked by default on first boot even when they are
> unavailable and again are not the highest priority port. 

I think you should try to debug module-switch-on-port-available, that's
probably where the problem lies. But again, a pulseaudio verbose log is
the key here I'd say. I guess if you modify default.pa to not load the
restore modules that would emulate a clean boot well enough for this
scenario.

> For reference the bug that covers both the afore mentioned cases is:
> https://bugs.mageia.org/show_bug.cgi?id=11642 

I skimmed it through - if somebody has a "line out" port that's
unavailable but still he has something plugged into his line out, that's
a bug. Alsa-info and PA verbose log needed to figure out if it's PA or
kernel.

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] card: Only set active_profile with available profile

[David Henningsson]

On 11/18/2013 03:51 PM, Luiz Augusto von Dentz wrote:
> Im having a hard time to understand why any policy would prefer a
> profile that is less likely to work than another, 

Well, the "off" profile is the most likely to work, but it also gives
significantly less functionality than the other profiles... :-)

> but perhaps we are
> not willing the mess with other parts of the code that were using this
> information in a different way. But then why we are bothering
> selecting any profile on pa_card_new? This is a policy decision and it
> seems we are not able to do a generic one so it should not even try,
> pick the first and be done with it.
> 
> Anyway if pa_card_new do not select 'off' by default it pretty much
> breaks Bluetooth qualification 

I'm not sure what you mean with "qualification", but it sounds like
quite a bad bug in the bluetooth code, if it can't handle that policy
modules setting the policy to anything but "off" when the card is added?

> and if I change the priority of 'off'
> profile to force it to be selected it breaks module-bluetooth-policy.



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
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

[Luiz Augusto von Dentz]

Hi Tanu,

On Mon, Nov 18, 2013 at 4:32 PM, Tanu Kaskinen
 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
>>  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] card: Only set active_profile with available profile

[Luiz Augusto von Dentz]

Hi Tanu,

On Mon, Nov 18, 2013 at 9:25 AM, Tanu Kaskinen
 wrote:
> On Sun, 2013-11-17 at 17:53 +0200, Luiz Augusto von Dentz wrote:
>> Hi Colin, Tanu,
>>
>> On Sun, Nov 17, 2013 at 3:17 PM, Colin Guthrie  wrote:
>> > 'Twas brillig, and Tanu Kaskinen at 16/11/13 08:51 did gyre and gimble:
>> >> On Fri, 2013-11-15 at 09:31 +0100, Colin Guthrie wrote:
>> >> I haven't concentrated on the patches enough to even know what
>> >> problem you're solving - is it just that the assertion makes you
>> >> nervous, or is there some other benefit.
>> >
>> > I'm not solving any specific problem per-se. I'm just looking at several
>> > bugs where incorrect profiles appear to be selected by default on first
>> > boot where HDMI is picked over regular analog output (for those alsa
>> > cards which share a single card for both HDMI and Analog output rather
>> > than having it presented as separate cards) despite HDMI having a lower
>> > priority. This lead me to look at this code and got me thinking - this
>> > is the only reason for the patch.
>> >
>> > A further complication to my current bugs is that sometimes Headphone
>> > *ports* are picked by default on first boot even when they are
>> > unavailable and again are not the highest priority port. It seems like
>> > the same problem but in a different, but similar, part of the code. This
>> > is why the second set of patches applied the same logical fixes to the
>> > Sink/Source code that Luiz added to the Card profiles. I think it makes
>> > sense to keep these bits of code approximately in sync regardless of the
>> > outcome of this discussion - the same logic should apply.
>>
>> I can try and apply the same logic to Sink/Source, it just that
>> overall this kind of policy should be in one place or then remove it
>> altogether and make sure the list is always ordered by their module so
>> the core should just pick the first one, the problem we were trying to
>> solve was that there is no hook for priority changes thus they are
>> considered static which is probably why we are relying on
>> pa_card_set_profile_available +
>> PA_CORE_HOOK_CARD_PROFILE_AVAILABLE_CHANGED to indicate the profiles
>> changes.
>>
>> So perhaps we should came to an agreement where and how we should be
>> doing this sort of policy, we could perhaps generalize
>> module-bluetooth-policy, or do we want to continue with per technology
>> policy?
>
> I don't think the policy in module-bluetooth-policy is useful outside
> Bluetooth. module-bluetooth-policy is mainly concerned with the
> transport state in its policy, and the profile availability is only used
> as a proxy for the transport state. I'd be happy if
> module-bluetooth-policy would be changed so that instead of using the
> profile states, it would directly track the transport states using the
> APIs that pa_bluez_discovery provides. But since using the profile
> states as a proxy works fine too, I'm not really asking anyone to do
> anything about that.
>
> From the proposed changes, I think Colin's patches 1 and 3 make sense.
> Preferring PA_AVAILABLE_YES over PA_AVAILABLE_UNKNOWN in generic code
> isn't a good idea.

Im having a hard time to understand why any policy would prefer a
profile that is less likely to work than another, but perhaps we are
not willing the mess with other parts of the code that were using this
information in a different way. But then why we are bothering
selecting any profile on pa_card_new? This is a policy decision and it
seems we are not able to do a generic one so it should not even try,
pick the first and be done with it.

Anyway if pa_card_new do not select 'off' by default it pretty much
breaks Bluetooth qualification and if I change the priority of 'off'
profile to force it to be selected it breaks module-bluetooth-policy.

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

[Tanu Kaskinen]

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

[Luiz Augusto von Dentz]

Hi Tanu,

On Mon, Nov 18, 2013 at 9:36 AM, Tanu Kaskinen
 wrote:
> On Tue, 2013-11-12 at 13:02 +0200, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz 
>>
>> 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