[PATCH 2/2] volume: Add LFE balance API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux