I disagree with "Also there would not be any feature/functional changes in pulseaudio when older bluez version is in use". Tests prove that there's no reason for this, if only one mode/profile is used. JP 07.10.2019, 15:36, "Pali Rohár" <pali.rohar@xxxxxxxxx>: > I will try to look what is the problem without --experimental. This > should work correctly prior merging this PR. > > Need to --experimental is just temporary until support in bluez is not > fully stable. I guess it would be non-experimental in next bluez > version. > > Also there would not be any feature/functional changes in pulseaudio > when older bluez version is in use. So also no change in automatic mode > for older bluez versions. > > On Monday 07 October 2019 15:30:49 Hyperion wrote: >> Without the --experimental flag : negociation stops (see hcidump below) and device falls back to "Off" status. >> >> btw : if you implement my (simple) negociation algorithm for the "Automatic Quality" mode ; AND make it to work without --experimental Bluez flag : it woiuld be close to perfect. >> >> See my latest patch https://gitlab.freedesktop.org/Hyperion/pulseaudio/tree/SBC-XQ >> >> jp >> >> 07.10.2019, 15:25, "Pali Rohár" <pali.rohar@xxxxxxxxx>: >> > And what happened without --experimental? >> > >> > Aim is to support all bluez versions also with and without >> > --experimental flag. Just for older versions (or without experimental) >> > it should behave like before this patch series -- only SBC codec in >> > automatic mode and no codec switching. >> > >> > On Monday 07 October 2019 15:20:35 Hyperion wrote: >> >> Thanks, I forgot the "--experimental" param. >> >> >> >> Works as expected, like the previous v12 serie of patches >> >> >> >> JP >> >> >> >> 07.10.2019, 15:15, "Pali Rohár" <pali.rohar@xxxxxxxxx>: >> >> > Can you check if you have Bluez 5.51 and bluetoothd daemon is running with --experimental param? >> >> > >> >> > And is not it possible to change profile from Off to Automatic Quality? >> >> > >> >> > On Monday 07 October 2019 15:08:59 Hyperion wrote: >> >> >> The 10 patches compile OK on PA master without warnings. >> >> >> >> >> >> But doesn't work (device is Off with "Automatic Quality unavailable" status). >> >> >> >> >> >> Tested on 2 devices. >> >> >> >> >> >> hcidump avdtp >> >> >> HCI sniffer - Bluetooth packet analyzer ver 5.51 >> >> >> device: hci0 snap_len: 1500 filter: 0x400 >> >> >> < AVDTP(s): Discover cmd: transaction 9 nsp 0x00 >> >> >> > AVDTP(s): Discover rsp: transaction 9 nsp 0x00 >> >> >> ACP SEID 1 - Audio Sink >> >> >> ACP SEID 2 - Audio Sink >> >> >> ACP SEID 3 - Audio Sink >> >> >> < AVDTP(s): Set config cmd: transaction 10 nsp 0x00 >> >> >> ACP SEID 1 - INT SEID 2 >> >> >> Media Transport >> >> >> Media Codec - SBC >> >> >> 44.1kHz >> >> >> DualChannel >> >> >> 16 Blocks >> >> >> 8 Subbands >> >> >> Loudness >> >> >> Bitpool Range 38-38 >> >> >> > AVDTP(s): Set config rsp: transaction 10 nsp 0x00 >> >> >> < AVDTP(s): Open cmd: transaction 11 nsp 0x00 >> >> >> ACP SEID 1 >> >> >> > AVDTP(s): Open rsp: transaction 11 nsp 0x00 >> >> >> >> >> >> 06.10.2019, 19:59, "Pali Rohár" <pali.rohar@xxxxxxxxx>: >> >> >> > Previously module-bluetooth-policy was switching from A2DP to HSP profile >> >> >> > when VOIP application started recording of source. Now it switch to profile >> >> >> > with the highest priority which has both sink and source. In most cases it >> >> >> > is HSP profile, but it can be also bi-directional A2DP profile (e.g. >> >> >> > FastStream codec) as it has better audio quality. >> >> >> > --- >> >> >> > src/modules/bluetooth/module-bluetooth-policy.c | 123 ++++++++++++------------ >> >> >> > 1 file changed, 62 insertions(+), 61 deletions(-) >> >> >> > >> >> >> > diff --git a/src/modules/bluetooth/module-bluetooth-policy.c b/src/modules/bluetooth/module-bluetooth-policy.c >> >> >> > index 04313aa84..9652a91fe 100644 >> >> >> > --- a/src/modules/bluetooth/module-bluetooth-policy.c >> >> >> > +++ b/src/modules/bluetooth/module-bluetooth-policy.c >> >> >> > @@ -59,7 +59,12 @@ struct userdata { >> >> >> > pa_hook_slot *card_init_profile_slot; >> >> >> > pa_hook_slot *card_unlink_slot; >> >> >> > pa_hook_slot *profile_available_changed_slot; >> >> >> > - pa_hashmap *will_need_revert_card_map; >> >> >> > + pa_hashmap *profile_switch_map; >> >> >> > +}; >> >> >> > + >> >> >> > +struct profile_switch { >> >> >> > + const char *from_profile; >> >> >> > + const char *to_profile; >> >> >> > }; >> >> >> > >> >> >> > /* When a source is created, loopback it to default sink */ >> >> >> > @@ -142,43 +147,57 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void * >> >> >> > return PA_HOOK_OK; >> >> >> > } >> >> >> > >> >> >> > -static void card_set_profile(struct userdata *u, pa_card *card, bool revert_to_a2dp) >> >> >> > -{ >> >> >> > +static void card_set_profile(struct userdata *u, pa_card *card, const char *revert_to_profile_name) { >> >> >> > + pa_card_profile *iter_profile; >> >> >> > pa_card_profile *profile; >> >> >> > + struct profile_switch *ps; >> >> >> > + char *old_profile_name; >> >> >> > void *state; >> >> >> > >> >> >> > - /* Find available profile and activate it */ >> >> >> > - PA_HASHMAP_FOREACH(profile, card->profiles, state) { >> >> >> > - if (profile->available == PA_AVAILABLE_NO) >> >> >> > - continue; >> >> >> > - >> >> >> > - /* Check for correct profile based on revert_to_a2dp */ >> >> >> > - if (revert_to_a2dp) { >> >> >> > - if (!pa_startswith(profile->name, "a2dp_sink")) >> >> >> > + if (revert_to_profile_name) { >> >> >> > + profile = pa_hashmap_get(card->profiles, revert_to_profile_name); >> >> >> > + } else { >> >> >> > + /* Find highest priority profile with both sink and source */ >> >> >> > + profile = NULL; >> >> >> > + PA_HASHMAP_FOREACH(iter_profile, card->profiles, state) { >> >> >> > + if (iter_profile->available == PA_AVAILABLE_NO) >> >> >> > continue; >> >> >> > - } else { >> >> >> > - if (!pa_streq(profile->name, "headset_head_unit")) >> >> >> > + if (iter_profile->n_sources == 0 || iter_profile->n_sinks == 0) >> >> >> > continue; >> >> >> > + if (!profile || profile->priority < iter_profile->priority) >> >> >> > + profile = iter_profile; >> >> >> > } >> >> >> > + } >> >> >> > >> >> >> > - pa_log_debug("Setting card '%s' to profile '%s'", card->name, profile->name); >> >> >> > + if (!profile) { >> >> >> > + pa_log_warn("Could not find any suitable profile for card '%s'", card->name); >> >> >> > + return; >> >> >> > + } >> >> >> > >> >> >> > - if (pa_card_set_profile(card, profile, false) != 0) { >> >> >> > - pa_log_warn("Could not set profile '%s'", profile->name); >> >> >> > - continue; >> >> >> > - } >> >> >> > + old_profile_name = card->active_profile->name; >> >> >> > + >> >> >> > + pa_log_debug("Setting card '%s' from profile '%s' to profile '%s'", card->name, old_profile_name, profile->name); >> >> >> > >> >> >> > - /* When we are not in revert_to_a2dp phase flag this card for will_need_revert */ >> >> >> > - if (!revert_to_a2dp) >> >> >> > - pa_hashmap_put(u->will_need_revert_card_map, card, PA_INT_TO_PTR(1)); >> >> >> > + if (pa_card_set_profile(card, profile, false) != 0) { >> >> >> > + pa_log_warn("Could not set profile '%s'", profile->name); >> >> >> > + return; >> >> >> > + } >> >> >> > >> >> >> > - break; >> >> >> > + /* When not reverting, store data for future reverting */ >> >> >> > + if (!revert_to_profile_name) { >> >> >> > + ps = pa_xnew0(struct profile_switch, 1); >> >> >> > + ps->from_profile = old_profile_name; >> >> >> > + ps->to_profile = profile->name; >> >> >> > + pa_hashmap_put(u->profile_switch_map, card, ps); >> >> >> > } >> >> >> > } >> >> >> > >> >> >> > /* Switch profile for one card */ >> >> >> > -static void switch_profile(pa_card *card, bool revert_to_a2dp, void *userdata) { >> >> >> > +static void switch_profile(pa_card *card, bool revert, void *userdata) { >> >> >> > struct userdata *u = userdata; >> >> >> > + struct profile_switch *ps; >> >> >> > + const char *from_profile; >> >> >> > + const char *to_profile; >> >> >> > const char *s; >> >> >> > >> >> >> > /* Only consider bluetooth cards */ >> >> >> > @@ -186,29 +205,25 @@ static void switch_profile(pa_card *card, bool revert_to_a2dp, void *userdata) { >> >> >> > if (!s || !pa_streq(s, "bluetooth")) >> >> >> > return; >> >> >> > >> >> >> > - if (revert_to_a2dp) { >> >> >> > - /* In revert_to_a2dp phase only consider cards with will_need_revert flag and remove it */ >> >> >> > - if (!pa_hashmap_remove(u->will_need_revert_card_map, card)) >> >> >> > + if (revert) { >> >> >> > + /* In revert phase only consider cards which switched profile */ >> >> >> > + if (!(ps = pa_hashmap_remove(u->profile_switch_map, card))) >> >> >> > return; >> >> >> > >> >> >> > - /* Skip card if does not have active hsp profile */ >> >> >> > - if (!pa_streq(card->active_profile->name, "headset_head_unit")) >> >> >> > - return; >> >> >> > + from_profile = ps->from_profile; >> >> >> > + to_profile = ps->to_profile; >> >> >> > + pa_xfree(ps); >> >> >> > >> >> >> > - /* Skip card if already has active a2dp profile */ >> >> >> > - if (pa_startswith(card->active_profile->name, "a2dp_sink")) >> >> >> > + /* Skip card if does not have active profile to which was switched */ >> >> >> > + if (!pa_streq(card->active_profile->name, to_profile)) >> >> >> > return; >> >> >> > } else { >> >> >> > - /* Skip card if does not have active a2dp profile */ >> >> >> > - if (!pa_startswith(card->active_profile->name, "a2dp_sink")) >> >> >> > - return; >> >> >> > - >> >> >> > - /* Skip card if already has active hsp profile */ >> >> >> > - if (pa_streq(card->active_profile->name, "headset_head_unit")) >> >> >> > + /* Skip card if already has both sink and source */ >> >> >> > + if (card->active_profile->n_sources > 0 && card->active_profile->n_sinks > 0) >> >> >> > return; >> >> >> > } >> >> >> > >> >> >> > - card_set_profile(u, card, revert_to_a2dp); >> >> >> > + card_set_profile(u, card, revert ? from_profile : NULL); >> >> >> > } >> >> >> > >> >> >> > /* Return true if we should ignore this source output */ >> >> >> > @@ -254,15 +269,15 @@ static unsigned source_output_count(pa_core *c, void *userdata) { >> >> >> > } >> >> >> > >> >> >> > /* Switch profile for all cards */ >> >> >> > -static void switch_profile_all(pa_idxset *cards, bool revert_to_a2dp, void *userdata) { >> >> >> > +static void switch_profile_all(pa_idxset *cards, bool revert, void *userdata) { >> >> >> > pa_card *card; >> >> >> > uint32_t idx; >> >> >> > >> >> >> > PA_IDXSET_FOREACH(card, cards, idx) >> >> >> > - switch_profile(card, revert_to_a2dp, userdata); >> >> >> > + switch_profile(card, revert, userdata); >> >> >> > } >> >> >> > >> >> >> > -/* When a source output is created, switch profile a2dp to profile hsp */ >> >> >> > +/* When a source output is created, switch profile to some which has both sink and source */ >> >> >> > static pa_hook_result_t source_output_put_hook_callback(pa_core *c, pa_source_output *source_output, void *userdata) { >> >> >> > pa_assert(c); >> >> >> > pa_assert(source_output); >> >> >> > @@ -274,7 +289,7 @@ static pa_hook_result_t source_output_put_hook_callback(pa_core *c, pa_source_ou >> >> >> > return PA_HOOK_OK; >> >> >> > } >> >> >> > >> >> >> > -/* When all source outputs are unlinked, switch profile hsp back back to profile a2dp */ >> >> >> > +/* When all source outputs are unlinked, switch to previous profile */ >> >> >> > static pa_hook_result_t source_output_unlink_hook_callback(pa_core *c, pa_source_output *source_output, void *userdata) { >> >> >> > pa_assert(c); >> >> >> > pa_assert(source_output); >> >> >> > @@ -291,30 +306,16 @@ static pa_hook_result_t source_output_unlink_hook_callback(pa_core *c, pa_source >> >> >> > } >> >> >> > >> >> >> > static pa_hook_result_t card_init_profile_hook_callback(pa_core *c, pa_card *card, void *userdata) { >> >> >> > - struct userdata *u = userdata; >> >> >> > - const char *s; >> >> >> > - >> >> >> > pa_assert(c); >> >> >> > pa_assert(card); >> >> >> > >> >> >> > + /* If there are not some source outputs do nothing */ >> >> >> > if (source_output_count(c, userdata) == 0) >> >> >> > return PA_HOOK_OK; >> >> >> > >> >> >> > - /* Only consider bluetooth cards */ >> >> >> > - s = pa_proplist_gets(card->proplist, PA_PROP_DEVICE_BUS); >> >> >> > - if (!s || !pa_streq(s, "bluetooth")) >> >> >> > - return PA_HOOK_OK; >> >> >> > - >> >> >> > - /* Ignore card if has already set other initial profile than a2dp */ >> >> >> > - if (card->active_profile && >> >> >> > - !pa_startswith(card->active_profile->name, "a2dp_sink")) >> >> >> > - return PA_HOOK_OK; >> >> >> > - >> >> >> > - /* Set initial profile to hsp */ >> >> >> > - card_set_profile(u, card, false); >> >> >> > + /* Set initial profile to some with source */ >> >> >> > + switch_profile(card, false, userdata); >> >> >> > >> >> >> > - /* Flag this card for will_need_revert */ >> >> >> > - pa_hashmap_put(u->will_need_revert_card_map, card, PA_INT_TO_PTR(1)); >> >> >> > return PA_HOOK_OK; >> >> >> > } >> >> >> > >> >> >> > @@ -447,7 +448,7 @@ int pa__init(pa_module *m) { >> >> >> > goto fail; >> >> >> > } >> >> >> > >> >> >> > - u->will_need_revert_card_map = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func); >> >> >> > + u->profile_switch_map = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func); >> >> >> > >> >> >> > u->source_put_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_SOURCE_PUT], PA_HOOK_NORMAL, >> >> >> > (pa_hook_cb_t) source_put_hook_callback, u); >> >> >> > @@ -512,7 +513,7 @@ void pa__done(pa_module *m) { >> >> >> > if (u->profile_available_changed_slot) >> >> >> > pa_hook_slot_free(u->profile_available_changed_slot); >> >> >> > >> >> >> > - pa_hashmap_free(u->will_need_revert_card_map); >> >> >> > + pa_hashmap_free(u->profile_switch_map); >> >> >> > >> >> >> > pa_xfree(u); >> >> >> > } >> >> >> > -- >> >> >> > 2.11.0 >> >> >> > >> >> >> > _______________________________________________ >> >> >> > pulseaudio-discuss mailing list >> >> >> > pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx >> >> >> > https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss >> >> >> _______________________________________________ >> >> >> pulseaudio-discuss mailing list >> >> >> pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx >> >> >> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss >> >> > >> >> > -- >> >> > Pali Rohár >> >> > pali.rohar@xxxxxxxxx >> > >> > -- >> > Pali Rohár >> > pali.rohar@xxxxxxxxx > > -- > Pali Rohár > pali.rohar@xxxxxxxxx _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
