Thanks for the patch! Before I'll review the patch, can you resubmit it using "git send-email"? The patch doesn't apply in its current form, because lines are wrapped (there might be other formatting issues as well). "git send-email" is guaranteed to produce well-formatted patches. Instructions here: https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/ Alternatively, you can also submit the patch as a merge request in GitLab. Some comments about the commit message: * The title (first line) should have a "bluetooth:" prefix. If you look at the PulseAudio commit history, you see that all commit titles have some prefix. * I'd replace the term "absolute volume" with "hardware volume". * It would be nice have a problem statement, so that someone who isn't already very familiar with the topic can understand what practical problem is being fixed. * Since the patch is rather large, it would be good to have an overview of what the patch does. * "Matching disable absolute volume database" isn't a proper sentence, and it's pretty hard to understand. English isn't your first language, but please do your best to communicate clearly. I'm guessing that you meant something like this: The patch includes a database of devices with bad volume behaviour. Hardware volume control is disabled for those devices. The database originates from Android: [insert URL here] * "Also fixes source volume setting." What was broken in the source volume setting? -- Tanu On Sat, 2018-10-20 at 22:50 +0800, EHfive wrote: > Patch v1: > > https://lists.freedesktop.org/archives/pulseaudio-discuss/2018-October/030538.html > > ValdikSS <iam@xxxxxxxxxxxxxxx> requests > > https://lists.freedesktop.org/archives/pulseaudio-discuss/2018-October/030566.html > > > Matching disable absolute volume database. > > Also fixes source volume setting. > > > src/modules/bluetooth/bluez5-util.c | 65 > +++++++++++++++++++++++++++++++++++++++++ > src/modules/bluetooth/bluez5-util.h | 4 +++ > src/modules/bluetooth/module-bluez5-device.c | 203 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 272 insertions(+) > > ======================================== > > diff --git a/src/modules/bluetooth/bluez5-util.c > b/src/modules/bluetooth/bluez5-util.c > index 2d83373..72cd05a 100644 > --- a/src/modules/bluetooth/bluez5-util.c > +++ b/src/modules/bluetooth/bluez5-util.c > @@ -348,6 +348,50 @@ void > pa_bluetooth_transport_free(pa_bluetooth_transport *t) { > pa_xfree(t); > } > > +static int bluez5_transport_set_property(pa_bluetooth_transport *t, > const char *prop_name, int prop_type, void *prop_value){ > + DBusMessage *m, *r; > + DBusError err; > + DBusMessageIter i; > + const char * interface = BLUEZ_MEDIA_TRANSPORT_INTERFACE; > + > + pa_log_debug("Setting property, Owner: %s; Path: %s; Property: > %s",t->owner, t->path, prop_name); > + > + pa_assert(t); > + pa_assert(t->device); > + pa_assert(t->device->discovery); > + > + dbus_error_init(&err); > + > + pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, > "org.freedesktop.DBus.Properties", "Set")); > + > + dbus_message_iter_init_append(m, &i); > + > + pa_assert_se(dbus_message_iter_append_basic(&i, DBUS_TYPE_STRING, > &interface)); > + pa_assert_se(dbus_message_iter_append_basic(&i, DBUS_TYPE_STRING, > &prop_name)); > + pa_dbus_append_basic_variant(&i, prop_type, prop_value); > + > + r = > dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), > m, -1, &err); > + dbus_message_unref(m); > + m = NULL; > + if(r) { > + dbus_message_unref(r); > + r = NULL; > + } > + > + if(dbus_error_is_set(&err)) { > + pa_log_debug("Failed to set property \"%s.%s\"", interface, > prop_name); > + return -1; > + } > + > + return 0; > +} > + > +static int bluez5_transport_set_volume(pa_bluetooth_transport *t, > uint16_t volume){ > + if(t->a2dp_gain == volume) > + return 0; > + return bluez5_transport_set_property(t, "Volume", DBUS_TYPE_UINT16, > &volume); > +} > + > static int bluez5_transport_acquire_cb(pa_bluetooth_transport *t, bool > optional, size_t *imtu, size_t *omtu) { > DBusMessage *m, *r; > DBusError err; > @@ -441,6 +485,14 @@ bool > pa_bluetooth_device_any_transport_connected(const pa_bluetooth_device *d) { > return false; > } > > +void pa_transport_set_a2dp_gain(pa_bluetooth_transport *t, uint16_t > a2dp_gain){ > + if(t->a2dp_gain == a2dp_gain) > + return; > + t->a2dp_gain = a2dp_gain; > + pa_hook_fire(pa_bluetooth_discovery_hook(t->device->discovery, > PA_BLUETOOTH_HOOK_TRANSPORT_A2DP_GAIN_CHANGED), t); > +} > + > + > static int transport_state_from_string(const char* value, > pa_bluetooth_transport_state_t *state) { > pa_assert(value); > pa_assert(state); > @@ -483,6 +535,18 @@ static void > parse_transport_property(pa_bluetooth_transport *t, DBusMessageIter > pa_bluetooth_transport_set_state(t, state); > } > > + break; > + } > + case DBUS_TYPE_UINT16: { > + > + uint16_t value; > + dbus_message_iter_get_basic(&variant_i, &value); > + > + if (pa_streq(key, "Volume")) { > + pa_log_debug("Transport Volume Changed to %u ", value); > + pa_transport_set_a2dp_gain(t, value); > + } > + > break; > } > } > @@ -1468,6 +1532,7 @@ static DBusMessage > *endpoint_set_configuration(DBusConnection *conn, DBusMessage > t = pa_bluetooth_transport_new(d, sender, path, p, config, size); > t->acquire = bluez5_transport_acquire_cb; > t->release = bluez5_transport_release_cb; > + t->set_volume = bluez5_transport_set_volume; > pa_bluetooth_transport_put(t); > > pa_log_debug("Transport %s available for profile %s", t->path, > pa_bluetooth_profile_to_string(t->profile)); > diff --git a/src/modules/bluetooth/bluez5-util.h > b/src/modules/bluetooth/bluez5-util.h > index ad30708..5b8149d 100644 > --- a/src/modules/bluetooth/bluez5-util.h > +++ b/src/modules/bluetooth/bluez5-util.h > @@ -47,6 +47,7 @@ typedef enum pa_bluetooth_hook { > PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED, /* Call > data: pa_bluetooth_transport */ > PA_BLUETOOTH_HOOK_TRANSPORT_MICROPHONE_GAIN_CHANGED, /* Call > data: pa_bluetooth_transport */ > PA_BLUETOOTH_HOOK_TRANSPORT_SPEAKER_GAIN_CHANGED, /* Call > data: pa_bluetooth_transport */ > + PA_BLUETOOTH_HOOK_TRANSPORT_A2DP_GAIN_CHANGED, /* Call data: > pa_bluetooth_transport */ > PA_BLUETOOTH_HOOK_MAX > } pa_bluetooth_hook_t; > > @@ -70,6 +71,7 @@ typedef void > (*pa_bluetooth_transport_release_cb)(pa_bluetooth_transport *t); > typedef void > (*pa_bluetooth_transport_destroy_cb)(pa_bluetooth_transport *t); > typedef void > (*pa_bluetooth_transport_set_speaker_gain_cb)(pa_bluetooth_transport *t, > uint16_t gain); > typedef void > (*pa_bluetooth_transport_set_microphone_gain_cb)(pa_bluetooth_transport > *t, uint16_t gain); > +typedef int > (*pa_bluetooth_transport_set_volume_cb)(pa_bluetooth_transport *t, > uint16_t volume); > > struct pa_bluetooth_transport { > pa_bluetooth_device *device; > @@ -84,6 +86,7 @@ struct pa_bluetooth_transport { > > uint16_t microphone_gain; > uint16_t speaker_gain; > + uint16_t a2dp_gain; > > pa_bluetooth_transport_state_t state; > > @@ -92,6 +95,7 @@ struct pa_bluetooth_transport { > pa_bluetooth_transport_destroy_cb destroy; > pa_bluetooth_transport_set_speaker_gain_cb set_speaker_gain; > pa_bluetooth_transport_set_microphone_gain_cb set_microphone_gain; > + pa_bluetooth_transport_set_volume_cb set_volume; > void *userdata; > }; > > diff --git a/src/modules/bluetooth/module-bluez5-device.c > b/src/modules/bluetooth/module-bluez5-device.c > index 351ad12..4b4c9b2 100644 > --- a/src/modules/bluetooth/module-bluez5-device.c > +++ b/src/modules/bluetooth/module-bluez5-device.c > @@ -27,6 +27,8 @@ > #include <arpa/inet.h> > #include <sbc/sbc.h> > > +#include <bluetooth/bluetooth.h> > + > #include <pulse/rtclock.h> > #include <pulse/timeval.h> > #include <pulse/utf8.h> > @@ -64,6 +66,7 @@ PA_MODULE_USAGE("path=<device object path>" > #define BITPOOL_DEC_LIMIT 32 > #define BITPOOL_DEC_STEP 5 > #define HSP_MAX_GAIN 15 > +#define BLUEZ_MAX_GAIN 127 > > static const char* const valid_modargs[] = { > "path", > @@ -113,6 +116,7 @@ struct userdata { > pa_hook_slot *transport_state_changed_slot; > pa_hook_slot *transport_speaker_gain_changed_slot; > pa_hook_slot *transport_microphone_gain_changed_slot; > + pa_hook_slot *transport_a2dp_gain_changed_slot; > > pa_bluetooth_discovery *discovery; > pa_bluetooth_device *device; > @@ -1056,6 +1060,37 @@ static void source_set_volume_cb(pa_source *s) { > u->transport->set_microphone_gain(u->transport, gain); > } > > +static void source_set_a2dp_volume_cb(pa_source *s) { > + uint16_t gain; > + pa_volume_t volume; > + struct userdata *u; > + > + pa_assert(s); > + pa_assert(s->core); > + > + u = s->userdata; > + > + pa_assert(u); > + pa_assert(u->source == s); > + > + if (u->transport->set_volume == NULL) > + return; > + > + gain = (uint16_t) ((pa_cvolume_max(&s->real_volume) * > BLUEZ_MAX_GAIN) / PA_VOLUME_NORM); > + > + pa_log_debug("Real Volume Gain:%u", gain); > + > + if (gain > BLUEZ_MAX_GAIN) > + gain = BLUEZ_MAX_GAIN; > + > + volume = (pa_volume_t) (gain * PA_VOLUME_NORM / BLUEZ_MAX_GAIN); > + > + pa_cvolume_set(&s->real_volume, u->sample_spec.channels, volume); > + pa_cvolume_set(&s->soft_volume, u->sample_spec.channels, volume); > + > + u->transport->set_volume(u->transport, gain); > +} > + > /* Run from main thread */ > static int add_source(struct userdata *u) { > pa_source_new_data data; > @@ -1109,6 +1144,9 @@ static int add_source(struct userdata *u) { > if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || > u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) { > pa_source_set_set_volume_callback(u->source, > source_set_volume_cb); > u->source->n_volume_steps = 16; > + } else if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE) { > + pa_source_set_set_volume_callback(u->source, > source_set_a2dp_volume_cb); > + u->source->n_volume_steps = 1; > } > return 0; > } > @@ -1230,6 +1268,110 @@ static void sink_set_volume_cb(pa_sink *s) { > u->transport->set_speaker_gain(u->transport, gain); > } > > +static void sink_set_a2dp_volume_cb(pa_sink *s) { > + uint16_t gain; > + pa_volume_t volume; > + struct userdata *u; > + > + pa_assert(s); > + pa_assert(s->core); > + > + u = s->userdata; > + > + pa_assert(u); > + pa_assert(u->sink == s); > + > + if (u->transport->set_volume == NULL) > + return; > + > + gain = (uint16_t) ((pa_cvolume_max(&s->real_volume) * > BLUEZ_MAX_GAIN) / PA_VOLUME_NORM); > + > + pa_log_debug("Real Volume Gain:%u", gain); > + > + if (gain > BLUEZ_MAX_GAIN) > + gain = BLUEZ_MAX_GAIN; > + > + volume = (pa_volume_t) (gain * PA_VOLUME_NORM / BLUEZ_MAX_GAIN); > + > + pa_cvolume_set(&s->real_volume, u->sample_spec.channels, volume); > + > + if(u->transport->set_volume(u->transport, gain) < 0) { > + pa_cvolume_set(&s->soft_volume, u->sample_spec.channels, > PA_VOLUME_NORM); > + pa_sink_set_set_volume_callback(s, NULL); > + u->transport->a2dp_gain = 0xFFu; > + } > +} > + > +static bool disable_absolute_volume_match(const char * bt_addr_str) { > + bdaddr_t bt_addr; > + int i; > + static bdaddr_t affected_bt_addr_cache; > + static bool effected_bt_addr_cached = false; > + > + static const struct { > + bdaddr_t addr; > + size_t length; > + } database[] = { > + > + // Ausdom M05 - unacceptably loud volume > + {{{0xa0, 0xe9, 0xdb, 0, 0, 0}}, 3}, > + // iKross IKBT83B HS - unacceptably loud volume > + {{{0x00, 0x14, 0x02, 0, 0, 0}}, 3}, > + // JayBird BlueBuds X - low granularity on volume control > + {{{0x44, 0x5e, 0xf3, 0, 0, 0}}, 3}, > + {{{0xd4, 0x9c, 0x28, 0, 0, 0}}, 3}, > + // LG Tone HBS-730 - unacceptably loud volume > + {{{0x00, 0x18, 0x6b, 0, 0, 0}}, 3}, > + {{{0xb8, 0xad, 0x3e, 0, 0, 0}}, 3}, > + // LG Tone HV-800 - unacceptably loud volume > + {{{0xa0, 0xe9, 0xdb, 0, 0, 0}}, 3}, > + // Motorola Roadster > + {{{0x00, 0x24, 0x1C, 0, 0, 0}}, 3}, > + // Mpow Cheetah - unacceptably loud volume > + {{{0x00, 0x11, 0xb1, 0, 0, 0}}, 3}, > + // SOL REPUBLIC Tracks Air - unable to adjust volume back > off from max > + {{{0xa4, 0x15, 0x66, 0, 0, 0}}, 3}, > + // Swage Rokitboost HS - unacceptably loud volume > + {{{0x00, 0x14, 0xf1, 0, 0, 0}}, 3}, > + // VW Car Kit - not enough granularity with volume > + {{{0x00, 0x26, 0x7e, 0, 0, 0}}, 3}, > + {{{0x90, 0x03, 0xb7, 0, 0, 0}}, 3}, > + // deepblue2 - cannot change smoothly the volume, 0x b/37834035 > + {{{0x0c, 0xa6, 0x94, 0, 0, 0}}, 3} > + }; > + > + for (i = 0; i < 6; ++i, bt_addr_str += 3) > + bt_addr.b[i] = (uint8_t) strtol(bt_addr_str, NULL, 16); > + > + if (effected_bt_addr_cached){ > + int j; > + bool flag = true; > + for (j = 0; j < 6; ++j) > + if (affected_bt_addr_cache.b[j] != bt_addr.b[j]){ > + flag = false; > + break; > + } > + if(flag) > + return true; > + } > + > + for(i = 0; i < PA_ELEMENTSOF(database); ++i){ > + int j; > + bool flag = true; > + for(j = 0; j < database[i].length; ++j) > + if(database[i].addr.b[j] != bt_addr.b[j]){ > + flag = false; > + break; > + } > + if(flag){ > + affected_bt_addr_cache = bt_addr; > + effected_bt_addr_cached = true; > + return true; > + } > + } > + return false; > +} > + > /* Run from main thread */ > static int add_sink(struct userdata *u) { > pa_sink_new_data data; > @@ -1284,6 +1426,11 @@ static int add_sink(struct userdata *u) { > if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || > u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) { > pa_sink_set_set_volume_callback(u->sink, sink_set_volume_cb); > u->sink->n_volume_steps = 16; > + } else if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) { > + if(disable_absolute_volume_match(u->device->address)) > + return 0; > + pa_sink_set_set_volume_callback(u->sink, sink_set_a2dp_volume_cb); > + u->sink->n_volume_steps = 1; > } > return 0; > } > @@ -2340,6 +2487,56 @@ static pa_hook_result_t > transport_microphone_gain_changed_cb(pa_bluetooth_discov > return PA_HOOK_OK; > } > > +static pa_hook_result_t > transport_a2dp_gain_changed_cb(pa_bluetooth_discovery *y, > pa_bluetooth_transport *t, struct userdata *u) { > + pa_volume_t volume; > + pa_cvolume v; > + uint16_t gain; > + > + pa_assert(t); > + pa_assert(u); > + > + if (t != u->transport) > + return PA_HOOK_OK; > + > + if(disable_absolute_volume_match(u->device->address)) > + return PA_HOOK_OK; > + > + gain = t->a2dp_gain; > + volume = (pa_volume_t) (gain * PA_VOLUME_NORM / BLUEZ_MAX_GAIN); > + > + pa_cvolume_set(&v, u->sample_spec.channels, volume); > + > + if(u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK){ > + pa_assert(u->sink); > + > + if(!u->sink->set_volume){ > + pa_cvolume_set(&u->sink->soft_volume, > u->sample_spec.channels, PA_VOLUME_NORM); > + pa_sink_set_set_volume_callback(u->sink, > sink_set_a2dp_volume_cb); > + } > + > + /* Mute when gain equal 0 or 1 */ > + if (gain <= 1) > + pa_sink_mute_changed(u->sink, true); > + else if(u->sink->muted) > + pa_sink_mute_changed(u->sink, false); > + > + pa_sink_volume_changed(u->sink, &v); > + } else if(u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE){ > + pa_assert(u->source); > + > + /* Mute when gain equal 0 or 1 */ > + if (gain <= 1) > + pa_source_mute_changed(u->source, true); > + else if(u->source->muted) > + pa_source_mute_changed(u->source, false); > + > + pa_source_set_volume(u->source, &v, true, true); > + } > + > + > + return PA_HOOK_OK; > +} > + > /* Run from main thread context */ > static int device_process_msg(pa_msgobject *obj, int code, void *data, > int64_t offset, pa_memchunk *chunk) { > struct bluetooth_msg *m = BLUETOOTH_MSG(obj); > @@ -2430,6 +2627,9 @@ int pa__init(pa_module* m) { > u->transport_microphone_gain_changed_slot = > pa_hook_connect(pa_bluetooth_discovery_hook(u->discovery, > PA_BLUETOOTH_HOOK_TRANSPORT_MICROPHONE_GAIN_CHANGED), PA_HOOK_NORMAL, > (pa_hook_cb_t) transport_microphone_gain_changed_cb, u); > > + u->transport_a2dp_gain_changed_slot = > + pa_hook_connect(pa_bluetooth_discovery_hook(u->discovery, > PA_BLUETOOTH_HOOK_TRANSPORT_A2DP_GAIN_CHANGED), PA_HOOK_NORMAL, > (pa_hook_cb_t) transport_a2dp_gain_changed_cb, u); > + > if (add_card(u) < 0) > goto fail; > > @@ -2491,6 +2691,9 @@ void pa__done(pa_module *m) { > if (u->transport_microphone_gain_changed_slot) > pa_hook_slot_free(u->transport_microphone_gain_changed_slot); > > + if (u->transport_a2dp_gain_changed_slot) > + pa_hook_slot_free(u->transport_a2dp_gain_changed_slot); > + > if (u->sbc_info.buffer) > pa_xfree(u->sbc_info.buffer); > > > _______________________________________________ > 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
