[pulseaudio-discuss] [PATCH 2/2] volume: Add LFE balance API
David Henningsson
david.henningsson at canonical.com
Tue Oct 27 01:02:07 PDT 2015
Thanks for the review. I agree with your review comments and will start
working on a v2 shortly.
// David
On 2015-10-27 06:27, Tanu Kaskinen wrote:
> On Fri, 2015-09-18 at 14:47 +0200, David Henningsson wrote:
>> The gnome/unity-control-center UIs have a master volume slider, and
>> three sub-sliders: balance, fade, and subwoofer. Balance and fade
>> use PA's set_balance and set_fade APIs accordingly, but the subwoofer
>> slider sometimes does unintuitive things.
>>
>> In order to make that slider behave better, let's add a LFE balance
>> API that these volume control UIs can use instead. With this API,
>> the UI can balance between "no subwoofer" and "only subwoofer" with
>> "equal balance" in the middle, which would make it more consistent
>> with the behaviour of the other sliders.
>>
>> BugLink: https://bugzilla.gnome.org/show_bug.cgi?id=753847
>>
>> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
>> ---
>> src/map-file | 3 +++
>> src/pulse/channelmap.c | 13 +++++++++++++
>> src/pulse/channelmap.h | 5 +++++
>> src/pulse/volume.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> src/pulse/volume.h | 18 ++++++++++++++++++
>> 5 files changed, 79 insertions(+)
>>
>> diff --git a/src/map-file b/src/map-file
>> index 5159829..93a62b8 100644
>> --- a/src/map-file
>> +++ b/src/map-file
>> @@ -7,6 +7,7 @@ pa_bytes_snprint;
>> pa_bytes_to_usec;
>> pa_channel_map_can_balance;
>> pa_channel_map_can_fade;
>> +pa_channel_map_can_lfe_balance;
>> pa_channel_map_compatible;
>> pa_channel_map_equal;
>> pa_channel_map_has_position;
>> @@ -127,6 +128,7 @@ pa_cvolume_dec;
>> pa_cvolume_equal;
>> pa_cvolume_get_balance;
>> pa_cvolume_get_fade;
>> +pa_cvolume_get_lfe_balance;
>> pa_cvolume_get_position;
>> pa_cvolume_inc;
>> pa_cvolume_inc_clamp;
>> @@ -142,6 +144,7 @@ pa_cvolume_scale_mask;
>> pa_cvolume_set;
>> pa_cvolume_set_balance;
>> pa_cvolume_set_fade;
>> +pa_cvolume_set_lfe_balance;
>> pa_cvolume_set_position;
>> pa_cvolume_snprint;
>> pa_cvolume_snprint_verbose;
>> diff --git a/src/pulse/channelmap.c b/src/pulse/channelmap.c
>> index c342ef6..ab343bc 100644
>> --- a/src/pulse/channelmap.c
>> +++ b/src/pulse/channelmap.c
>> @@ -684,6 +684,19 @@ int pa_channel_map_can_fade(const pa_channel_map *map) {
>> (PA_CHANNEL_POSITION_MASK_REAR & m);
>> }
>>
>> +int pa_channel_map_can_lfe_balance(const pa_channel_map *map) {
>> + pa_channel_position_mask_t l, m;
>> +
>> + pa_assert(map);
>> + pa_return_val_if_fail(pa_channel_map_valid(map), 0);
>> +
>> + m = pa_channel_map_mask(map);
>> + l = PA_CHANNEL_POSITION_MASK(PA_CHANNEL_POSITION_LFE);
>> +
>> + return
>> + (l & m) && ((~l) & m);
>> +}
>
> I think aux channels shouldn't be considered. Possibly mono (a.k.a.
> unspecified) channels should be excluded too (it's quite possible that
> mono channels never coexist with lfe channels, though, so this doesn't
> matter much).
>
>> +
>> const char* pa_channel_map_to_name(const pa_channel_map *map) {
>> pa_bitset_t in_map[PA_BITSET_ELEMENTS(PA_CHANNEL_POSITION_MAX)];
>> unsigned c;
>> diff --git a/src/pulse/channelmap.h b/src/pulse/channelmap.h
>> index 30904ef..6eabe20 100644
>> --- a/src/pulse/channelmap.h
>> +++ b/src/pulse/channelmap.h
>> @@ -338,6 +338,11 @@ int pa_channel_map_can_balance(const pa_channel_map *map) PA_GCC_PURE;
>> * there are front/rear channels available. \since 0.9.15 */
>> int pa_channel_map_can_fade(const pa_channel_map *map) PA_GCC_PURE;
>>
>> +/** Returns non-zero if it makes sense to apply a volume 'lfe balance'
>> + * (i.e.\ 'balance' between LFE and non-LFE channels) with this mapping,
>> + * i.e.\ if there are LFE and non-LFE channels available. \since 8.0 */
>> +int pa_channel_map_can_lfe_balance(const pa_channel_map *map) PA_GCC_PURE;
>> +
>> /** Tries to find a well-known channel mapping name for this channel
>> * mapping, i.e.\ "stereo", "surround-71" and so on. If the channel
>> * mapping is unknown NULL will be returned. This name can be parsed
>> diff --git a/src/pulse/volume.c b/src/pulse/volume.c
>> index 2b34ded..91906be 100644
>> --- a/src/pulse/volume.c
>> +++ b/src/pulse/volume.c
>> @@ -552,6 +552,10 @@ static bool on_center(pa_channel_position_t p) {
>> return !!(PA_CHANNEL_POSITION_MASK(p) & PA_CHANNEL_POSITION_MASK_CENTER);
>> }
>>
>> +static bool on_hfe(pa_channel_position_t p) {
>> + return !(p == PA_CHANNEL_POSITION_LFE);
>> +}
>
> The aux/mono question applies here too.
>
>> +
>> static bool on_lfe(pa_channel_position_t p) {
>> return p == PA_CHANNEL_POSITION_LFE;
>> }
>> @@ -829,6 +833,42 @@ pa_cvolume* pa_cvolume_set_fade(pa_cvolume *v, const pa_channel_map *map, float
>> return set_balance(v, map, new_fade, on_rear, on_front);
>> }
>>
>> +float pa_cvolume_get_lfe_balance(const pa_cvolume *v, const pa_channel_map *map) {
>> + pa_volume_t hfe, lfe;
>> +
>> + pa_assert(v);
>> + pa_assert(map);
>> +
>> + pa_return_val_if_fail(pa_cvolume_compatible_with_channel_map(v, map), 0.0f);
>> +
>> + if (!pa_channel_map_can_lfe_balance(map))
>> + return 0.0f;
>> +
>> + get_avg(map, v, &hfe, &lfe, on_hfe, on_lfe);
>> +
>> + if (hfe == lfe)
>> + return 0.0f;
>> +
>> + if (hfe > lfe)
>> + return -1.0f + ((float) lfe / (float) hfe);
>> + else
>> + return 1.0f - ((float) hfe / (float) lfe);
>> +}
>> +
>> +pa_cvolume* pa_cvolume_set_lfe_balance(pa_cvolume *v, const pa_channel_map *map, float new_balance) {
>> + pa_assert(map);
>> + pa_assert(v);
>> +
>> + pa_return_val_if_fail(pa_cvolume_compatible_with_channel_map(v, map), NULL);
>> + pa_return_val_if_fail(new_balance >= -1.0f, NULL);
>> + pa_return_val_if_fail(new_balance <= 1.0f, NULL);
>> +
>> + if (!pa_channel_map_can_lfe_balance(map))
>> + return v;
>> +
>> + return set_balance(v, map, new_balance, on_hfe, on_lfe);
>> +}
>> +
>> pa_cvolume* pa_cvolume_set_position(
>> pa_cvolume *cv,
>> const pa_channel_map *map,
>> diff --git a/src/pulse/volume.h b/src/pulse/volume.h
>> index ec777b2..d749dda 100644
>> --- a/src/pulse/volume.h
>> +++ b/src/pulse/volume.h
>> @@ -372,6 +372,24 @@ float pa_cvolume_get_fade(const pa_cvolume *v, const pa_channel_map *map) PA_GCC
>> * pa_channel_map_can_fade(). \since 0.9.15 */
>> pa_cvolume* pa_cvolume_set_fade(pa_cvolume *v, const pa_channel_map *map, float new_fade);
>>
>> +/** Calculate a 'lfe balance' value for the specified volume with
>> + * the specified channel map. The return value will range from
>> + * -1.0f (no lfe) to +1.0f (only lfe), where 0.0f is balanced.
>> + * If no value is applicable to this channel map the return value
>> + * will always be 0.0f. See pa_channel_map_can_fade(). \since 8.0 */
>
> Copy-paste error:
> s/pa_channel_map_can_fade/pa_channel_map_can_lfe_balance/
>
>> +float pa_cvolume_get_lfe_balance(const pa_cvolume *v, const pa_channel_map *map) PA_GCC_PURE;
>
> --
> Tanu
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
More information about the pulseaudio-discuss
mailing list