9 files changed, 177 insertions, 94 deletions

diff --git a/firmware/drivers/audio/sdl.c b/firmware/drivers/audio/sdl.c
index 9bb399b910..ae66380d16 100644
--- a/firmware/drivers/audio/sdl.c
+++ b/firmware/drivers/audio/sdl.c
@@ -23,36 +23,48 @@
 #include "config.h"
 #include "sound.h"
 #include "pcm_sampr.h"
+#ifdef HAVE_SW_VOLUME_CONTROL
+#include "pcm_sw_volume.h"
+#include "fixedpoint.h"
+#endif
 
 /**
- * Audio Hardware api. Make them do nothing as we cannot properly simulate with
- * SDL. if we used DSP we would run code that doesn't actually run on the target
+ * Audio Hardware api. Make some of them do nothing as we cannot properly
+ * simulate with SDL. if we used DSP we would run code that doesn't actually
+ * run on the target
  **/
-
 #ifdef HAVE_SW_VOLUME_CONTROL
-#include "pcm_sw_volume.h"
-
-void audiohw_set_volume(int vol_l, int vol_r)
+static int sdl_volume_level(int volume)
 {
-    pcm_set_master_volume(vol_l, vol_r);
+    int shift = (1 - sound_numdecimals(SOUND_VOLUME)) << 16;
+    int minvol = fp_mul(sound_min(SOUND_VOLUME), fp_exp10(shift, 16), 16);
+    return volume <= minvol ? INT_MIN : volume;
 }
+#endif /* HAVE_SW_VOLUME_CONTROL */
 
-#else /* ndef HAVE_SW_VOLUME_CONTROL */
-
-
+#if defined(AUDIOHW_HAVE_MONO_VOLUME)
 void audiohw_set_volume(int volume)
 {
-#if CONFIG_CODEC == SWCODEC
+#ifdef HAVE_SW_VOLUME_CONTROL
+    volume = sdl_volume_level(volume);
+    pcm_set_master_volume(volume, volume);
+#elif CONFIG_CODEC == SWCODEC
     extern void pcm_set_mixer_volume(int volume);
     pcm_set_mixer_volume(volume);
 #endif
     (void)volume;
 }
-#endif /* HAVE_SW_VOLUME_CONTROL */
-
-/**
- * stubs here, for the simulator
- **/
+#else /* !AUDIOHW_HAVE_MONO_VOLUME */
+void audiohw_set_volume(int vol_l, int vol_r)
+{
+#ifdef HAVE_SW_VOLUME_CONTROL
+    vol_l = sdl_volume_level(vol_l);
+    vol_r = sdl_volume_level(vol_r);
+    pcm_set_master_volume(vol_l, vol_r);
+#endif
+    (void)vol_l; (void)vol_r;
+}
+#endif /* AUDIOHW_HAVE_MONO_VOLUME */
 
 #if defined(AUDIOHW_HAVE_PRESCALER)
 void audiohw_set_prescaler(int value)
@@ -62,7 +74,8 @@ void audiohw_set_prescaler(int value)
 #endif
     (void)value;
 }
-#endif
+#endif /* AUDIOHW_HAVE_PRESCALER */
+
 #if defined(AUDIOHW_HAVE_BALANCE)
 void audiohw_set_balance(int value)     { (void)value; }
 #endif
diff --git a/firmware/export/audiohw.h b/firmware/export/audiohw.h
index 843ac0c0c4..4379c20a79 100644
--- a/firmware/export/audiohw.h
+++ b/firmware/export/audiohw.h
@@ -111,11 +111,6 @@ struct sound_settings_info
 #include "hosted_codec.h"
 #endif
 
-#if defined(SIMULATOR) && !defined(HAVE_SW_VOLUME_CONTROL)
-/* For now, without software volume control, sim only supports mono control */
-#define AUDIOHW_HAVE_MONO_VOLUME
-#endif
-
 /* convert caps into defines */
 #ifdef AUDIOHW_CAPS
 /* Tone controls */
diff --git a/firmware/export/config.h b/firmware/export/config.h
index 6a4a4648c8..7d7a18cc23 100644
--- a/firmware/export/config.h
+++ b/firmware/export/config.h
@@ -1139,6 +1139,24 @@ Lyre prototype 1 */
 #define ROCKBOX_HAS_LOGDISKF
 #endif
 
+#if defined(HAVE_SDL_AUDIO) \
+    && !(CONFIG_PLATFORM & PLATFORM_MAEMO5) \
+    && !defined(HAVE_SW_VOLUME_CONTROL) \
+    && CONFIG_CODEC == SWCODEC
+/* SW volume is needed for accurate control and no double buffering should be
+ * required. If target uses SW volume, then its definitions are used instead
+ * so things are as on target. */
+#define HAVE_SW_VOLUME_CONTROL
+#define PCM_SW_VOLUME_UNBUFFERED /* pcm driver itself is buffered */
+#ifdef SIMULATOR
+/* For sim, nice res for ~ -127dB..+36dB that so far covers all targets */
+#define PCM_SW_VOLUME_FRACBITS  (24)
+#else
+/* For app, use fractional-only setup for -79..+0, no large-integer math */
+#define PCM_SW_VOLUME_FRACBITS  (16)
+#endif /* SIMULATOR */
+#endif /* default SDL SW volume conditions */
+
 /* null audiohw setting macro for when codec header is included for reasons
    other than audio support */
 #define AUDIOHW_SETTING(name, us, nd, st, minv, maxv, defv, expr...)
diff --git a/firmware/export/hosted_codec.h b/firmware/export/hosted_codec.h
index 72495709e8..f5e92ed297 100644
--- a/firmware/export/hosted_codec.h
+++ b/firmware/export/hosted_codec.h
@@ -21,8 +21,13 @@
 #ifndef HOSTED_CODEC_H
 #define HOSTED_CODEC_H
 
+#if defined(HAVE_SDL_AUDIO) \
+    && !(CONFIG_PLATFORM & PLATFORM_MAEMO5)
+AUDIOHW_SETTING(VOLUME,      "dB",   0,  1, -80,   0,   0)
+#else
 #define AUDIOHW_CAPS    (MONO_VOL_CAP)
 AUDIOHW_SETTING(VOLUME,      "dB",   0,  1, -99,   0,   0)
+#endif /* CONFIG_PLATFORM & PLATFORM_SDL */
 
 #if (CONFIG_PLATFORM & PLATFORM_ANDROID)
 /* Bass and treble tone controls */
diff --git a/firmware/export/pcm-internal.h b/firmware/export/pcm-internal.h
index 03e5c5e6e7..2b73f64ef6 100644
--- a/firmware/export/pcm-internal.h
+++ b/firmware/export/pcm-internal.h
@@ -27,13 +27,36 @@
 #ifdef HAVE_SW_VOLUME_CONTROL
 /* Default settings - architecture may have other optimal values */
 
-#define PCM_FACTOR_BITS    15 /* Allows -73 to +6dB gain, sans 64-bit math */
+#ifndef PCM_SW_VOLUME_FRACBITS
+/* Allows -73 to +6dB gain, sans large integer math */
+#define PCM_SW_VOLUME_FRACBITS  (15)
+#endif
+
+/* Constants selected based on integer math overflow avoidance */
+#if PCM_SW_VOLUME_FRACBITS <= 16
+#define PCM_FACTOR_MAX      0x00010000u
+#define PCM_FACTOR_UNITY    (1u << PCM_SW_VOLUME_FRACBITS)
+#elif PCM_SW_VOLUME_FRACBITS <= 31
+#define PCM_FACTOR_MAX      0x80000000u
+#define PCM_FACTOR_UNITY    (1u << PCM_SW_VOLUME_FRACBITS)
+#endif /* PCM_SW_VOLUME_FRACBITS */
+
+#ifdef PCM_SW_VOLUME_UNBUFFERED
+/* Copies buffer with volume scaling applied */
+void pcm_sw_volume_copy_buffer(void *dst, const void *src, size_t size);
+#define pcm_copy_buffer pcm_sw_volume_copy_buffer
+#else /* !PCM_SW_VOLUME_UNBUFFERED */
+#ifdef HAVE_SDL_AUDIO
+#define pcm_copy_buffer memcpy
+#endif
+#ifndef PCM_PLAY_DBL_BUF_SAMPLES
 #define PCM_PLAY_DBL_BUF_SAMPLES 1024 /* Max 4KByte chunks */
+#endif
+#ifndef PCM_DBL_BUF_BSS
 #define PCM_DBL_BUF_BSS               /* In DRAM, uncached may be better */
-#define PCM_FACTOR_MIN        0x00000 /* Minimum final factor */
-#define PCM_FACTOR_MAX        0x10000 /* Maximum final factor */
+#endif
+#endif /* PCM_SW_VOLUME_UNBUFFERED */
 
-#define PCM_FACTOR_UNITY      (1 << PCM_FACTOR_BITS)
 #endif /* HAVE_SW_VOLUME_CONTROL */
 
 #define PCM_SAMPLE_SIZE     (2 * sizeof (int16_t))
@@ -84,22 +107,22 @@ static FORCE_INLINE enum pcm_dma_status pcm_play_call_status_cb(
 static FORCE_INLINE enum pcm_dma_status
 pcm_play_dma_status_callback(enum pcm_dma_status status)
 {
-#ifdef HAVE_SW_VOLUME_CONTROL
+#if defined(HAVE_SW_VOLUME_CONTROL) && !defined(PCM_SW_VOLUME_UNBUFFERED)
     extern enum pcm_dma_status
         pcm_play_dma_status_callback_int(enum pcm_dma_status status);
     return pcm_play_dma_status_callback_int(status);
 #else
     return pcm_play_call_status_cb(status);
-#endif /* HAVE_SW_VOLUME_CONTROL */
+#endif /* HAVE_SW_VOLUME_CONTROL && !PCM_SW_VOLUME_UNBUFFERED */
 }
 
-#ifdef HAVE_SW_VOLUME_CONTROL
+#if defined(HAVE_SW_VOLUME_CONTROL) && !defined(PCM_SW_VOLUME_UNBUFFERED)
 void pcm_play_dma_start_int(const void *addr, size_t size);
 void pcm_play_dma_pause_int(bool pause);
 void pcm_play_dma_stop_int(void);
 void pcm_play_stop_int(void);
 const void *pcm_play_dma_get_peak_buffer_int(int *count);
-#endif /* HAVE_SW_VOLUME_CONTROL */
+#endif /* HAVE_SW_VOLUME_CONTROL && !PCM_SW_VOLUME_UNBUFFERED */
 
 /* Called by the bottom layer ISR when more data is needed. Returns true
  * if a new buffer is available, false otherwise. */
diff --git a/firmware/export/pcm_sw_volume.h b/firmware/export/pcm_sw_volume.h
index b5d70654a1..5088eadeb1 100644
--- a/firmware/export/pcm_sw_volume.h
+++ b/firmware/export/pcm_sw_volume.h
@@ -21,6 +21,12 @@
 #ifndef PCM_SW_VOLUME_H
 #define PCM_SW_VOLUME_H
 
+/***
+ ** Note: Only PCM drivers that are themselves buffered should use the
+ ** PCM_SW_VOLUME_UNBUFFERED configuration. This may be part of the platform,
+ ** the library or a hardware necessity. Normally, it shouldn't be used and
+ ** only the port developer can properly decide.
+ **/
 #ifdef HAVE_SW_VOLUME_CONTROL
 
 #include <audiohw.h>
diff --git a/firmware/pcm.c b/firmware/pcm.c
index 6bf0e12c8d..e095ab2cea 100644
--- a/firmware/pcm.c
+++ b/firmware/pcm.c
@@ -106,8 +106,9 @@ static void pcm_play_data_start_int(const void *addr, size_t size);
 static void pcm_play_pause_int(bool play);
 void pcm_play_stop_int(void);
 
-#ifndef HAVE_SW_VOLUME_CONTROL
-/** Standard hw volume control functions - otherwise, see pcm_sw_volume.c **/
+#if !defined(HAVE_SW_VOLUME_CONTROL) || defined(PCM_SW_VOLUME_UNBUFFERED)
+/** Standard hw volume/unbuffered control functions - otherwise, see
+ ** pcm_sw_volume.c **/
 static inline void pcm_play_dma_start_int(const void *addr, size_t size)
 {
     pcm_play_dma_start(addr, size);
@@ -150,7 +151,7 @@ bool pcm_play_dma_complete_callback(enum pcm_dma_status status,
     pcm_play_stop_int();
     return false;
 }
-#endif /* ndef HAVE_SW_VOLUME_CONTROL */
+#endif /* !HAVE_SW_VOLUME_CONTROL || PCM_SW_VOLUME_UNBUFFERED */
 
 static void pcm_play_data_start_int(const void *addr, size_t size)
 {
diff --git a/firmware/pcm_sw_volume.c b/firmware/pcm_sw_volume.c
index 473e63c7cb..eb77fe0c6b 100644
--- a/firmware/pcm_sw_volume.c
+++ b/firmware/pcm_sw_volume.c
@@ -26,63 +26,91 @@
 #include "fixedpoint.h"
 #include "pcm_sw_volume.h"
 
-/* source buffer from client */
-static const void * volatile src_buf_addr = NULL;
-static size_t volatile src_buf_rem = 0;
+/* volume factors set by pcm_set_master_volume */
+static uint32_t vol_factor_l = 0, vol_factor_r = 0;
 
-#define PCM_PLAY_DBL_BUF_SIZE (PCM_PLAY_DBL_BUF_SAMPLE*PCM_SAMPLE_SIZE)
-
-/* double buffer and frame length control */
-static int16_t pcm_dbl_buf[2][PCM_PLAY_DBL_BUF_SAMPLES*2]
-        PCM_DBL_BUF_BSS MEM_ALIGN_ATTR;
-static size_t pcm_dbl_buf_size[2];
-static int pcm_dbl_buf_num = 0;
-static size_t frame_size;
-static unsigned int frame_count, frame_err, frame_frac;
-
-static int32_t vol_factor_l = 0, vol_factor_r = 0;
 #ifdef AUDIOHW_HAVE_PRESCALER
-static int32_t prescale_factor = PCM_FACTOR_UNITY;
+/* prescale factor set by pcm_set_prescaler */
+static uint32_t prescale_factor = PCM_FACTOR_UNITY;
 #endif /* AUDIOHW_HAVE_PRESCALER */
 
-/* pcm scaling factors */
-static int32_t pcm_factor_l = 0, pcm_factor_r = 0;
-
-#define PCM_FACTOR_CLIP(f) \
-    MAX(MIN((f), PCM_FACTOR_MAX), PCM_FACTOR_MIN)
-#define PCM_SCALE_SAMPLE(f, s) \
-    (((f) * (s) + PCM_FACTOR_UNITY/2) >> PCM_FACTOR_BITS)
+/* final pcm scaling factors */
+static uint32_t pcm_factor_l = 0, pcm_factor_r = 0;
 
+/***
+ ** Volume scaling routine
+ ** If unbuffered, called externally by pcm driver
+ **/
 
 /* TODO: #include CPU-optimized routines and move this to /firmware/asm */
-static inline void pcm_copy_buffer(int16_t *dst, const int16_t *src,
-                                   size_t size)
+
+#if PCM_SW_VOLUME_FRACBITS <= 16
+#define PCM_F_T int32_t
+#else
+#define PCM_F_T int64_t /* Requires large integer math */
+#endif /* PCM_SW_VOLUME_FRACBITS */
+
+static inline int32_t pcm_scale_sample(PCM_F_T f, int32_t s)
 {
-    int32_t factor_l = pcm_factor_l;
-    int32_t factor_r = pcm_factor_r;
+    return (f * s + (PCM_F_T)PCM_FACTOR_UNITY/2) >> PCM_SW_VOLUME_FRACBITS;
+}
 
-    if (LIKELY(factor_l <= PCM_FACTOR_UNITY && factor_r <= PCM_FACTOR_UNITY))
+/* Copies buffer with volume scaling applied */
+#ifndef PCM_SW_VOLUME_UNBUFFERED
+static inline
+#endif
+void pcm_sw_volume_copy_buffer(void *dst, const void *src, size_t size)
+{
+    int16_t *d = dst;
+    const int16_t *s = src;
+    uint32_t factor_l = pcm_factor_l;
+    uint32_t factor_r = pcm_factor_r;
+
+    if (factor_l == PCM_FACTOR_UNITY && factor_r == PCM_FACTOR_UNITY)
+    {
+        /* Both unity */
+        memcpy(dst, src, size);
+    }
+    else if (LIKELY(factor_l <= PCM_FACTOR_UNITY &&
+                    factor_r <= PCM_FACTOR_UNITY))
     {
-        /* All cut or unity */
+        /* Either cut, both <= UNITY */
         while (size)
         {
-            *dst++ = PCM_SCALE_SAMPLE(factor_l, *src++);
-            *dst++ = PCM_SCALE_SAMPLE(factor_r, *src++);
+            *d++ = pcm_scale_sample(factor_l, *s++);
+            *d++ = pcm_scale_sample(factor_r, *s++);
             size -= PCM_SAMPLE_SIZE;
         }
     }
     else
     {
-        /* Any positive gain requires clipping */
+        /* Either positive gain, requires clipping */
         while (size)
         {
-            *dst++ = clip_sample_16(PCM_SCALE_SAMPLE(factor_l, *src++));
-            *dst++ = clip_sample_16(PCM_SCALE_SAMPLE(factor_r, *src++));
+            *d++ = clip_sample_16(pcm_scale_sample(factor_l, *s++));
+            *d++ = clip_sample_16(pcm_scale_sample(factor_r, *s++));
             size -= PCM_SAMPLE_SIZE;
         }
     }
 }
 
+#ifndef PCM_SW_VOLUME_UNBUFFERED
+/* source buffer from client */
+static const void * volatile src_buf_addr = NULL;
+static size_t volatile src_buf_rem = 0;
+
+#define PCM_PLAY_DBL_BUF_SIZE (PCM_PLAY_DBL_BUF_SAMPLE*PCM_SAMPLE_SIZE)
+
+/* double buffer and frame length control */
+static int16_t pcm_dbl_buf[2][PCM_PLAY_DBL_BUF_SAMPLES*2]
+        PCM_DBL_BUF_BSS MEM_ALIGN_ATTR;
+static size_t pcm_dbl_buf_size[2];
+static int pcm_dbl_buf_num = 0;
+static size_t frame_size;
+static unsigned int frame_count, frame_err, frame_frac;
+
+/** Overrides of certain functions in pcm.c and pcm-internal.h **/
+
 bool pcm_play_dma_complete_callback(enum pcm_dma_status status,
                                     const void **addr, size_t *size)
 {
@@ -155,7 +183,7 @@ pcm_play_dma_status_callback_int(enum pcm_dma_status status)
 
     pcm_dbl_buf_num ^= 1;
     pcm_dbl_buf_size[pcm_dbl_buf_num] = size;
-    pcm_copy_buffer(pcm_dbl_buf[pcm_dbl_buf_num], addr, size);
+    pcm_sw_volume_copy_buffer(pcm_dbl_buf[pcm_dbl_buf_num], addr, size);
 
     return PCM_DMAST_OK;
 }
@@ -216,27 +244,36 @@ const void * pcm_play_dma_get_peak_buffer_int(int *count)
     return NULL;
 }
 
+#endif /* PCM_SW_VOLUME_UNBUFFERED */
+
+
+/** Internal **/
+
 /* Return the scale factor corresponding to the centibel level */
-static int32_t pcm_centibels_to_factor(int volume)
+static uint32_t pcm_centibels_to_factor(int volume)
 {
     if (volume == PCM_MUTE_LEVEL)
         return 0; /* mute */
 
     /* Centibels -> fixedpoint */
-    return fp_factor(fp_div(volume, 10, PCM_FACTOR_BITS), PCM_FACTOR_BITS);
+    return (uint32_t)fp_factor(fp_div(volume, 10, PCM_SW_VOLUME_FRACBITS),
+                               PCM_SW_VOLUME_FRACBITS);
 }
 
+
+/** Public functions **/
+
 /* Produce final pcm scale factor */
 static void pcm_sync_prescaler(void)
 {
-    int32_t factor_l = vol_factor_l;
-    int32_t factor_r = vol_factor_r;
+    uint32_t factor_l = vol_factor_l;
+    uint32_t factor_r = vol_factor_r;
 #ifdef AUDIOHW_HAVE_PRESCALER
-    factor_l = fp_mul(prescale_factor, factor_l, PCM_FACTOR_BITS);
-    factor_r = fp_mul(prescale_factor, factor_r, PCM_FACTOR_BITS);
+    factor_l = fp_mul(prescale_factor, factor_l, PCM_SW_VOLUME_FRACBITS);
+    factor_r = fp_mul(prescale_factor, factor_r, PCM_SW_VOLUME_FRACBITS);
 #endif
-    pcm_factor_l = PCM_FACTOR_CLIP(factor_l);
-    pcm_factor_r = PCM_FACTOR_CLIP(factor_r);
+    pcm_factor_l = MIN(factor_l, PCM_FACTOR_MAX);
+    pcm_factor_r = MIN(factor_r, PCM_FACTOR_MAX);
 }
 
 #ifdef AUDIOHW_HAVE_PRESCALER
diff --git a/firmware/target/hosted/sdl/pcm-sdl.c b/firmware/target/hosted/sdl/pcm-sdl.c
index beefc7818c..290dffbb95 100644
--- a/firmware/target/hosted/sdl/pcm-sdl.c
+++ b/firmware/target/hosted/sdl/pcm-sdl.c
@@ -51,12 +51,6 @@
 extern bool debug_audio;
 #endif
 
-#ifdef HAVE_SW_VOLUME_CONTROL
-static int sim_volume = SDL_MIX_MAXVOLUME;
-#else
-static int sim_volume = 0;
-#endif
-
 #if CONFIG_CODEC == SWCODEC
 static int cvt_status = -1;
 
@@ -177,10 +171,10 @@ static void write_to_soundcard(struct pcm_udata *udata)
             cvt.len = rd * pcm_sample_bytes;
             cvt.buf = (Uint8 *) malloc(cvt.len * cvt.len_mult);
 
-            memcpy(cvt.buf, pcm_data, cvt.len);
+            pcm_copy_buffer(cvt.buf, pcm_data, cvt.len);
 
             SDL_ConvertAudio(&cvt);
-            SDL_MixAudio(udata->stream, cvt.buf, cvt.len_cvt, sim_volume);
+            memcpy(udata->stream, cvt.buf, cvt.len_cvt);
 
             udata->num_in = cvt.len / pcm_sample_bytes;
             udata->num_out = cvt.len_cvt / pcm_sample_bytes;
@@ -223,8 +217,8 @@ static void write_to_soundcard(struct pcm_udata *udata)
         }
     } else {
         udata->num_in = udata->num_out = MIN(udata->num_in, udata->num_out);
-        SDL_MixAudio(udata->stream, pcm_data, 
-                     udata->num_out * pcm_sample_bytes, sim_volume);
+        pcm_copy_buffer(udata->stream, pcm_data,
+                        udata->num_out * pcm_sample_bytes);
 #ifdef DEBUG
         if (udata->debug != NULL) {
            fwrite(pcm_data, sizeof(Uint8), udata->num_out * pcm_sample_bytes,
@@ -418,13 +412,4 @@ void pcm_play_dma_postinit(void)
 {
 }
 
-#ifndef HAVE_SW_VOLUME_CONTROL
-void pcm_set_mixer_volume(int volume)
-{
-    int minvol = sound_min(SOUND_VOLUME);
-    int volrange = sound_max(SOUND_VOLUME) - minvol;
-    sim_volume = SDL_MIX_MAXVOLUME * (volume / 10 - minvol) / volrange;
-}
-#endif /* HAVE_SW_VOLUME_CONTROL */
-
 #endif /* CONFIG_CODEC == SWCODEC */