pcm: improve 32-bit software volume scaling

rockbox-gerrit-noreply--- via rockbox-cvs <[email protected]>
Newsgroups gmane.comp.systems.archos.rockbox.cvs
Message-ID <[email protected]> 
commit 5b5cd252d025a19bc73edb3bc47e943798dbe334
Author: Aidan MacDonald <[email protected]>
Date:   Wed Feb 18 17:41:36 2026 +0000

    pcm: improve 32-bit software volume scaling
    
    Replace the factor calculation from pcm-alsa.c, which
    is based on signal *power* ratios, with the fp_factor()
    calculation that is based on amplitude ratios. Because
    power changes with the square of amplitude, this means
    1 dB of power equals 2 dB of amplitude.
    
    Rockbox's volume controls are amplitude-based, so the
    smallest step size for pcm-alsa was effectively 2 dB.
    The fp_factor() method supports 0.1 dB steps and is a
    bit more accurate besides, so it's simply superior in
    all respects (aside from taking a few more CPU cycles
    to calculate the factor).
    
    Change-Id: I34d143c225d8b5e085cde299fc405f83c13314bf

diff --git a/firmware/pcm_sw_volume.c b/firmware/pcm_sw_volume.c
index bee1559bb8..61a66d650a 100644
--- a/firmware/pcm_sw_volume.c
+++ b/firmware/pcm_sw_volume.c
@@ -51,22 +51,28 @@ static typeof (memcpy) *pcm_scaling_fn = NULL;
 
 /* take care of some defines for 32-bit software vol */
 #if (PCM_NATIVE_BITDEPTH > 16) /* >16-bit */
-
 # define HAVE_SWVOL_32
-# define PCM_VOL_SAMPLE_SIZE     (2 * sizeof (int32_t))
-# define PCM_DBL_BUF_SIZE_T int32_t
-
-# if !defined(PCM_DC_OFFSET_VALUE)
-/* PCM_DC_OFFSET_VALUE is only needed due to hardware quirk on Eros Q */
-#  define PCM_DC_OFFSET_VALUE 0
-# endif
-
+# define PCM_VOL_SAMPLE_SIZE    (2 * sizeof (int32_t))
+# define PCM_DBL_BUF_SIZE_T     int32_t
 #else /* 16-BIT */
+# define PCM_VOL_SAMPLE_SIZE    (2 * sizeof (int16_t))
+# define PCM_DBL_BUF_SIZE_T     int16_t
+#endif /* 16-BIT */
 
-# define PCM_VOL_SAMPLE_SIZE     PCM_SAMPLE_SIZE
-# define PCM_DBL_BUF_SIZE_T int16_t
+#if !defined(PCM_DC_OFFSET_VALUE)
+/* PCM_DC_OFFSET_VALUE is only needed due to hardware quirk on Eros Q */
+# define PCM_DC_OFFSET_VALUE 0
+#endif
 
-#endif /* 16-BIT */
+/*
+ * 16-bit samples are scaled up to an effective bit depth
+ * of (16+fracbits) bits and must be shifted to produce a
+ * native bit depth sample. Positive values correspond to
+ * right shifts (throwing away bits of the scaled result)
+ * while negative values correspond to left shifts (where
+ * the scaled result completely fits in the native sample).
+ */
+#define PCM_SCALE_SHIFT (16 + PCM_SW_VOLUME_FRACBITS - PCM_NATIVE_BITDEPTH)
 
 /***
  ** Volume scaling routines
@@ -83,10 +89,10 @@ static typeof (memcpy) *pcm_scaling_fn = NULL;
 /* Scale sample by PCM factor */
 static inline int32_t pcm_scale_sample(PCM_F_T f, int32_t s)
 {
-#if defined(HAVE_SWVOL_32)
-    return (f * s + PCM_DC_OFFSET_VALUE) >> (32 - PCM_NATIVE_BITDEPTH);
+#if PCM_SCALE_SHIFT > 0
+    return (f * s + PCM_DC_OFFSET_VALUE) >> PCM_SCALE_SHIFT;
 #else
-    return (f * s) >> PCM_SW_VOLUME_FRACBITS;
+    return (f * s + PCM_DC_OFFSET_VALUE) << (-PCM_SCALE_SHIFT);
 #endif
 }
 
@@ -347,28 +353,10 @@ static uint32_t pcm_centibels_to_factor(int volume)
 {
     if (volume == PCM_MUTE_LEVEL)
         return 0; /* mute */
-#if defined(HAVE_SWVOL_32)
-    /*
-     * 32-bit software volume taken from pcm-alsa.c
-     */
-    volume += 48; /* -42dB .. 0dB => 5dB .. 48dB */
-    /* NOTE if vol_dB = 5 then vol_shift = 1 but r = 1 so we do vol_shift - 1 >= 0
-     * otherwise vol_dB >= 0 implies vol_shift >= 2 so vol_shift - 2 >= 0 */
-    int vol_shift = volume / 3;
-    int r = volume % 3;
-    int32_t dig_vol_mult;
-    if(r == 0)
-        dig_vol_mult = 1 << vol_shift;
-    else if(r == 1)
-        dig_vol_mult = 1 << vol_shift | 1 << (vol_shift - 2);
-    else
-        dig_vol_mult = 1 << vol_shift | 1 << (vol_shift - 1);
-    return dig_vol_mult;
-#else /* standard software volume */
+
     /* Centibels -> fixedpoint */
     return (uint32_t)fp_factor(fp_div(volume, 10, PCM_SW_VOLUME_FRACBITS),
                                PCM_SW_VOLUME_FRACBITS);
-#endif /* HAVE_SWVOL_32 */
 }
 
 
diff --git a/firmware/target/hosted/aigo/erosqlinux_codec.c b/firmware/target/hosted/aigo/erosqlinux_codec.c
index 7cb1ef5f15..274d77ddb2 100644
--- a/firmware/target/hosted/aigo/erosqlinux_codec.c
+++ b/firmware/target/hosted/aigo/erosqlinux_codec.c
@@ -204,7 +204,7 @@ static void audiohw_set_volume_v1(int vol_l, int vol_r)
 
     int sw_volume_l = l <= min_pcm ? min_pcm : MIN(l, max_pcm);
     int sw_volume_r = r <= min_pcm ? min_pcm : MIN(r, max_pcm);
-    pcm_set_mixer_volume(sw_volume_l / 20, sw_volume_r / 20);
+    pcm_set_mixer_volume(sw_volume_l, sw_volume_r);
 }
 
 static void audiohw_set_volume_v2(int vol_l, int vol_r)
@@ -236,7 +236,7 @@ static void audiohw_set_volume_v2(int vol_l, int vol_r)
     alsa_controls_set_ints("Right Playback Volume", 1, &r);
 
     /* Dial back PCM mixer to avoid compression */
-    pcm_set_mixer_volume(global_settings.volume_limit / 2, global_settings.volume_limit / 2);
+    pcm_set_mixer_volume(global_settings.volume_limit, global_settings.volume_limit);
 }
 
 void audiohw_set_volume(int vol_l, int vol_r)
@@ -274,7 +274,7 @@ void audiohw_set_lineout_volume(int vol_l, int vol_r)
             } else {
                 int sw_volume_l = l <= min_pcm ? min_pcm : MIN(l, max_pcm);
                 int sw_volume_r = r <= min_pcm ? min_pcm : MIN(r, max_pcm);
-                pcm_set_mixer_volume(sw_volume_l / 20, sw_volume_r / 20);
+                pcm_set_mixer_volume(sw_volume_l, sw_volume_r);
             }
         }
     }
diff --git a/firmware/target/hosted/fiio/fiiolinux_codec.c b/firmware/target/hosted/fiio/fiiolinux_codec.c
index 99cff3f5e4..397e58286f 100644
--- a/firmware/target/hosted/fiio/fiiolinux_codec.c
+++ b/firmware/target/hosted/fiio/fiiolinux_codec.c
@@ -95,21 +95,21 @@ void audiohw_set_volume(int vol_l, int vol_r)
     if (ak_hw < 0)
        return;
 
-    vol[0] = vol_l / 20;
-    vol[1] = vol_r / 20;
+    vol[0] = vol_l;
+    vol[1] = vol_r;
 
     for (int i = 0; i < 2; i++)
     {
-        if (vol[i] > -56)
+        if (vol[i] > -1120)
         {
-            if (vol[i] < -12)
+            if (vol[i] < -240)
             {
                 vol_hw[i] = 1;
-                vol_sw[i] = vol[i] + 12;
+                vol_sw[i] = vol[i] + 240;
             }
             else
             {
-                vol_hw[i] = 25 - (-vol[i] * 2);
+                vol_hw[i] = 25 - (-vol[i] / 10);
                 vol_sw[i] = 0;
             }
         }
diff --git a/firmware/target/hosted/pcm-alsa.c b/firmware/target/hosted/pcm-alsa.c
index f6347cea73..9286369a25 100644
--- a/firmware/target/hosted/pcm-alsa.c
+++ b/firmware/target/hosted/pcm-alsa.c
@@ -50,6 +50,7 @@
 #include "pcm_sampr.h"
 #include "audiohw.h"
 #include "pcm-alsa.h"
+#include "fixedpoint.h"
 
 #include "logf.h"
 
@@ -267,46 +268,14 @@ error:
 }
 
 #if defined(HAVE_ALSA_32BIT)
-/* Digital volume explanation:
- * with very good approximation (<0.1dB) the convertion from dB to multiplicative
- * factor, for dB>=0, is 2^(dB/3). We can then notice that if we write dB=3*k+r
- * then this is 2^k*2^(r/3) so we only need to look at r=0,1,2. For r=0 this is
- * 1, for r=1 we have 2^(1/3)~=1.25 so we approximate by 1+1/4, and 2^(2/3)~=1.5
- * so we approximate by 1+1/2. To go from negative to nonnegative we notice that
- * 48 dB => 63095 factor ~= 2^16 so we virtually pre-multiply everything by 2^(-16)
- * and add 48dB to the input volume. We cannot go lower -43dB because several
- * values between -48dB and -43dB would require a fractional multiplier, which is
- * stupid to implement for such very low volume. */
-static int dig_vol_mult_l = 2 << 16; /* multiplicative factor to apply to each sample */
-static int dig_vol_mult_r = 2 << 16; /* multiplicative factor to apply to each sample */
+/* Multiplicative factors applied to each sample */
+static int32_t dig_vol_mult_l = 0;
+static int32_t dig_vol_mult_r = 0;
+
 void pcm_set_mixer_volume(int vol_db_l, int vol_db_r)
 {
-    if(vol_db_l > 0 || vol_db_r > 0 || vol_db_l < -43 || vol_db_r < -43)
-        panicf("invalid pcm alsa volume %d %d",   vol_db_l, vol_db_r);
-    if(format != SND_PCM_FORMAT_S32_LE)
-        panicf("this function assumes 32-bit sample size");
-    vol_db_l += 48; /* -42dB .. 0dB => 5dB .. 48dB */
-    vol_db_r += 48; /* -42dB .. 0dB => 5dB .. 48dB */
-    /* NOTE if vol_dB = 5 then vol_shift = 1 but r = 1 so we do vol_shift - 1 >= 0
-     * otherwise vol_dB >= 0 implies vol_shift >= 2 so vol_shift - 2 >= 0 */
-    int vol_shift_l = vol_db_l / 3;
-    int vol_shift_r = vol_db_r / 3;
-    int r_l = vol_db_l % 3;
-    int r_r = vol_db_r % 3;
-    if(r_l == 0)
-        dig_vol_mult_l = 1 << vol_shift_l;
-    else if(r_l == 1)
-        dig_vol_mult_l = 1 << vol_shift_l | 1 << (vol_shift_l - 2);
-    else
-        dig_vol_mult_l = 1 << vol_shift_l | 1 << (vol_shift_l - 1);
-    logf("l: %d dB -> factor = %d", vol_db_l - 48, dig_vol_mult_l);
-    if(r_r == 0)
-        dig_vol_mult_r = 1 << vol_shift_r;
-    else if(r_r == 1)
-        dig_vol_mult_r = 1 << vol_shift_r | 1 << (vol_shift_r - 2);
-    else
-        dig_vol_mult_r = 1 << vol_shift_r | 1 << (vol_shift_r - 1);
-    logf("r: %d dB -> factor = %d", vol_db_r - 48, dig_vol_mult_r);
+    dig_vol_mult_l = fp_factor(fp_div(vol_db_l, 10, 16), 16);
+    dig_vol_mult_r = fp_factor(fp_div(vol_db_r, 10, 16), 16);
 }
 #endif
 
diff --git a/firmware/target/hosted/pcm-alsa.h b/firmware/target/hosted/pcm-alsa.h
index f35be60074..3fa6ffe9db 100644
--- a/firmware/target/hosted/pcm-alsa.h
+++ b/firmware/target/hosted/pcm-alsa.h
@@ -21,10 +21,12 @@
 #define __PCM_ALSA_RB_H__
 
 #include <config.h>
+#include <limits.h>
 
 #if defined(HAVE_ALSA_32BIT)
+
 /* Set the PCM volume in dB: each sample with have this volume applied digitally
- * before being sent to ALSA. Volume must satisfy -43 <= dB <= 0 */
+ * before being sent to ALSA. Effective range -79 dB to 0 dB */
 void pcm_set_mixer_volume(int vol_db_l, int vol_db_r);
 #endif
 
diff --git a/firmware/target/hosted/sonynwz/nwzlinux-codec.c b/firmware/target/hosted/sonynwz/nwzlinux-codec.c
index 5085befb20..52ff9871bc 100644
--- a/firmware/target/hosted/sonynwz/nwzlinux-codec.c
+++ b/firmware/target/hosted/sonynwz/nwzlinux-codec.c
@@ -415,7 +415,7 @@ void audiohw_set_volume(int vol_l, int vol_r)
     printf(" set driver volume %d (%d dB)\n", drv_vol, curve->level[drv_vol] / 10);
     nwz_set_driver_vol(drv_vol);
     printf(" set digital volume %d dB\n", vol / 10);
-    pcm_set_mixer_volume(vol / 10, vol / 10);
+    pcm_set_mixer_volume(vol, vol);
 }
 
 void audiohw_close(void)
diff --git a/firmware/target/mips/ingenic_x1000/erosqnative/audiohw-erosqnative.c b/firmware/target/mips/ingenic_x1000/erosqnative/audiohw-erosqnative.c
index 353badaef8..3fc774cfdd 100644
--- a/firmware/target/mips/ingenic_x1000/erosqnative/audiohw-erosqnative.c
+++ b/firmware/target/mips/ingenic_x1000/erosqnative/audiohw-erosqnative.c
@@ -200,8 +200,8 @@ void audiohw_set_volume(int vol_l, int vol_r)
     }
     else /* PCM5102A */
     {
-        l = l <= PCM5102A_VOLUME_MIN ? PCM_MUTE_LEVEL : (l / 20);
-        r = r <= PCM5102A_VOLUME_MIN ? PCM_MUTE_LEVEL : (r / 20);
+        l = l <= PCM5102A_VOLUME_MIN ? PCM_MUTE_LEVEL : l;
+        r = r <= PCM5102A_VOLUME_MIN ? PCM_MUTE_LEVEL : r;
 
         pcm_set_master_volume(l, r);
     }
@@ -214,4 +214,4 @@ void audiohw_set_filter_roll_off(int value)
         es9018k2m_set_filter_roll_off(value);
     }
 }
-#endif /* !defined(BOOTLOADER) */
\ No newline at end of file
+#endif /* !defined(BOOTLOADER) */
-- 
rockbox-cvs mailing list
[email protected]
https://lists.haxx.se/mailman/listinfo/rockbox-cvs
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.