[pulseaudio-discuss] [PATCH next v0 3/8] bluetooth: Use ceil() to round up volume

[Tanu Kaskinen]

On Wed, 2012-12-05 at 17:23 +0100, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
> 
> The code can be simplified since it's just trying to round up the result
> of the division. Note that the resulting behavior is slightly different,
> specially when the volume is 0. In this case, it will remain at 0,
> instead of being set to 1.
> ---
>  src/modules/bluetooth/module-bluetooth-device.c | 25 +++++--------------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
> 
> diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
> index fdc621f..40b2fcf 100644
> --- a/src/modules/bluetooth/module-bluetooth-device.c
> +++ b/src/modules/bluetooth/module-bluetooth-device.c
> @@ -26,6 +26,7 @@
>  
>  #include <string.h>
>  #include <errno.h>
> +#include <math.h>
>  #include <linux/sockios.h>
>  #include <arpa/inet.h>
>  
> @@ -1330,21 +1331,13 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
>  
>          if (u->profile == PROFILE_HSP) {
>              if (u->sink && dbus_message_is_signal(m, "org.bluez.Headset", "SpeakerGainChanged")) {
> -                pa_volume_t volume = (pa_volume_t) (gain * PA_VOLUME_NORM / HSP_MAX_GAIN);
> -
> -                /* increment volume by one to correct rounding errors */
> -                if (volume < PA_VOLUME_NORM)
> -                    volume++;
> +                pa_volume_t volume = (pa_volume_t) ceil(gain * PA_VOLUME_NORM / HSP_MAX_GAIN);

The calculation inside ceil() is done with integers only, so the value
passed to ceil() will be an integer, and thus the function does nothing.

Also, I had forgot why there's any rounding error correction in the
first place. After checking the origin of the old rounding code, I now
know that the reason is that if the BT-to-PA volume conversion rounds
the result down (i.e. no explicit rounding), then BT-to-PA-to-BT
conversion will result in the BT volume being one step lower than what
the original value was. It would be nice to have a comment about this.

Hmm, I think we should actually use round() for rounding, at least for
the PA-to-BT conversion, because it doesn't make much sense UI-wise that
PA_VOLUME_NORM - 1 results in BT volume 14, when it's so close to the
maximum. Using round() in the PA-to-BT conversion would also fix the
original problem. I think it would make sense to use round() with the
BT-to-PA conversion too, just for extra correctness, even though in that
case it's mostly academic.

-- 
Tanu