From e189b33ff4cc530cb6e59a17b260675d7341e551 Mon Sep 17 00:00:00 2001 From: Michael Sevakis Date: Wed, 2 May 2012 20:53:07 -0400 Subject: [PATCH] Clean up peak calculating code. Mixer needn't keep peak data around that will never be used. Just pass pcm_peaks structure to it instead of allocating for every channel. Plugin API becomes incompatible. vu_meter digital mode was still using global peak calculation; switch it to playback channel like the rest. Remove some accumulated soil peaks inside pcm.c and make it more generic. Change-Id: Ib4d268d80b6a9d09915eea1c91eab483c1a2c009 --- apps/plugin.h | 6 +-- apps/plugins/oscilloscope.c | 4 +- apps/plugins/starfield.c | 6 ++- apps/plugins/vu_meter.c | 13 +++-- apps/recorder/peakmeter.c | 7 ++- firmware/export/pcm-internal.h | 7 --- firmware/export/pcm.h | 12 ++++- firmware/export/pcm_mixer.h | 2 +- firmware/pcm.c | 86 ++++++++++++++++------------------ firmware/pcm_mixer.c | 16 ++----- 10 files changed, 81 insertions(+), 78 deletions(-) diff --git a/apps/plugin.h b/apps/plugin.h index 588a01262e..4091cecc16 100644 --- a/apps/plugin.h +++ b/apps/plugin.h @@ -154,12 +154,12 @@ void* plugin_get_buffer(size_t *buffer_size); #define PLUGIN_MAGIC 0x526F634B /* RocK */ /* increase this every time the api struct changes */ -#define PLUGIN_API_VERSION 219 +#define PLUGIN_API_VERSION 220 /* update this to latest version if a change to the api struct breaks backwards compatibility (and please take the opportunity to sort in any new function which are "waiting" at the end of the function table) */ -#define PLUGIN_MIN_API_VERSION 219 +#define PLUGIN_MIN_API_VERSION 220 /* plugin return codes */ /* internal returns start at 0x100 to make exit(1..255) work */ @@ -697,7 +697,7 @@ struct plugin_api { const void * (*mixer_channel_get_buffer)(enum pcm_mixer_channel channel, int *count); void (*mixer_channel_calculate_peaks)(enum pcm_mixer_channel channel, - int *left, int *right); + struct pcm_peaks *peaks); void (*mixer_channel_play_data)(enum pcm_mixer_channel channel, pcm_play_callback_type get_more, const void *start, size_t size); diff --git a/apps/plugins/oscilloscope.c b/apps/plugins/oscilloscope.c index a4be0fbd8e..0bf951f069 100644 --- a/apps/plugins/oscilloscope.c +++ b/apps/plugins/oscilloscope.c @@ -881,8 +881,10 @@ enum plugin_status plugin_start(const void* parameter) left = rb->mas_codec_readreg(0xC); right = rb->mas_codec_readreg(0xD); #elif (CONFIG_CODEC == SWCODEC) + static struct pcm_peaks peaks; rb->mixer_channel_calculate_peaks(PCM_MIXER_CHAN_PLAYBACK, - &left, &right); + &peaks); + left = peaks.left; right = peaks.right; #endif if (osc.orientation == OSC_HORIZ) anim_horizontal(left, right); diff --git a/apps/plugins/starfield.c b/apps/plugins/starfield.c index 811e9c61ae..30b01b2645 100644 --- a/apps/plugins/starfield.c +++ b/apps/plugins/starfield.c @@ -228,9 +228,11 @@ static int plugin_main(void) /* Get the peaks. ( Borrowed from vu_meter ) */ #if (CONFIG_CODEC == SWCODEC) - int left_peak, right_peak; + static struct pcm_peaks peaks; rb->mixer_channel_calculate_peaks(PCM_MIXER_CHAN_PLAYBACK, - &left_peak, &right_peak); + &peaks); + #define left_peak peaks.left + #define right_peak peaks.right #else int left_peak = rb->mas_codec_readreg(0xC); int right_peak = rb->mas_codec_readreg(0xD); diff --git a/apps/plugins/vu_meter.c b/apps/plugins/vu_meter.c index 51f2d19f40..ec884442d4 100644 --- a/apps/plugins/vu_meter.c +++ b/apps/plugins/vu_meter.c @@ -704,9 +704,11 @@ static void analog_meter(void) { int left_peak = rb->mas_codec_readreg(0xC); int right_peak = rb->mas_codec_readreg(0xD); #elif (CONFIG_CODEC == SWCODEC) - int left_peak, right_peak; + static struct pcm_peaks peaks; rb->mixer_channel_calculate_peaks(PCM_MIXER_CHAN_PLAYBACK, - &left_peak, &right_peak); + &peaks); + #define left_peak peaks.left + #define right_peak peaks.right #endif if(vumeter_settings.analog_use_db_scale) { @@ -762,8 +764,11 @@ static void digital_meter(void) { int left_peak = rb->mas_codec_readreg(0xC); int right_peak = rb->mas_codec_readreg(0xD); #elif (CONFIG_CODEC == SWCODEC) - int left_peak, right_peak; - rb->pcm_calculate_peaks(&left_peak, &right_peak); + static struct pcm_peaks peaks; + rb->mixer_channel_calculate_peaks(PCM_MIXER_CHAN_PLAYBACK, + &peaks); + #define left_peak peaks.left + #define right_peak peaks.right #endif if(vumeter_settings.digital_use_db_scale) { diff --git a/apps/recorder/peakmeter.c b/apps/recorder/peakmeter.c index 9f0db3330a..48a695b933 100644 --- a/apps/recorder/peakmeter.c +++ b/apps/recorder/peakmeter.c @@ -619,8 +619,13 @@ void peak_meter_peek(void) /* read current values */ #if CONFIG_CODEC == SWCODEC if (pm_playback) + { + static struct pcm_peaks chan_peaks; /* *MUST* be static */ mixer_channel_calculate_peaks(PCM_MIXER_CHAN_PLAYBACK, - &pm_cur_left, &pm_cur_right); + &chan_peaks); + pm_cur_left = chan_peaks.left; + pm_cur_right = chan_peaks.right; + } #ifdef HAVE_RECORDING else pcm_calculate_rec_peaks(&pm_cur_left, &pm_cur_right); diff --git a/firmware/export/pcm-internal.h b/firmware/export/pcm-internal.h index abe3fe08dc..397cf6832f 100644 --- a/firmware/export/pcm-internal.h +++ b/firmware/export/pcm-internal.h @@ -29,13 +29,6 @@ ({ (start) = (void *)(((uintptr_t)(start) + 3) & ~3); \ (size) &= ~3; }) -struct pcm_peaks -{ - long period; - long tick; - uint16_t val[2]; -}; - void pcm_do_peak_calculation(struct pcm_peaks *peaks, bool active, const void *addr, int count); diff --git a/firmware/export/pcm.h b/firmware/export/pcm.h index df82c6092d..608129c078 100644 --- a/firmware/export/pcm.h +++ b/firmware/export/pcm.h @@ -21,7 +21,7 @@ #ifndef PCM_PLAYBACK_H #define PCM_PLAYBACK_H -#include /* size_t */ +#include /* size_t, uint32_t */ #include "config.h" enum pcm_dma_status @@ -71,6 +71,16 @@ void pcm_play_data(pcm_play_callback_type get_more, pcm_status_callback_type status_cb, const void *start, size_t size); +/* Kept internally for global PCM and used by mixer's verion of peak + calculation */ +struct pcm_peaks +{ + uint32_t left; /* Left peak value */ + uint32_t right; /* Right peak value */ + long period; /* For tracking calling period */ + long tick; /* Last tick called */ +}; + void pcm_calculate_peaks(int *left, int *right); const void* pcm_get_peak_buffer(int* count); size_t pcm_get_bytes_waiting(void); diff --git a/firmware/export/pcm_mixer.h b/firmware/export/pcm_mixer.h index 5d8deb2bbf..f6d212d8e0 100644 --- a/firmware/export/pcm_mixer.h +++ b/firmware/export/pcm_mixer.h @@ -111,7 +111,7 @@ const void * mixer_channel_get_buffer(enum pcm_mixer_channel channel, int *count /* Calculate peak values for channel */ void mixer_channel_calculate_peaks(enum pcm_mixer_channel channel, - int *left, int *right); + struct pcm_peaks *peaks); /* Adjust channel pointer by a given offset to support movable buffers */ void mixer_adjust_channel_address(enum pcm_mixer_channel channel, diff --git a/firmware/pcm.c b/firmware/pcm.c index 621ed56e0d..94b0d6eefb 100644 --- a/firmware/pcm.c +++ b/firmware/pcm.c @@ -102,9 +102,6 @@ unsigned long pcm_sampr SHAREDBSS_ATTR = HW_SAMPR_DEFAULT; /* samplerate frequency selection index */ int pcm_fsel SHAREDBSS_ATTR = HW_FREQ_DEFAULT; -/* peak data for the global peak values - i.e. what the final output is */ -static struct pcm_peaks global_peaks; - /* Called internally by functions to reset the state */ static void pcm_play_stopped(void) { @@ -125,42 +122,38 @@ static void pcm_wait_for_init(void) * * Used for recording and playback. */ -static void pcm_peak_peeker(const int32_t *addr, int count, uint16_t peaks[2]) +static void pcm_peak_peeker(const int16_t *p, int count, + struct pcm_peaks *peaks) { - int peak_l = 0, peak_r = 0; - const int32_t * const end = addr + count; + uint32_t peak_l = 0, peak_r = 0; + const int16_t *pend = p + 2 * count; do { - int32_t value = *addr; - int ch; + int32_t s; -#ifdef ROCKBOX_BIG_ENDIAN - ch = value >> 16; -#else - ch = (int16_t)value; -#endif - if (ch < 0) - ch = -ch; - if (ch > peak_l) - peak_l = ch; + s = p[0]; -#ifdef ROCKBOX_BIG_ENDIAN - ch = (int16_t)value; -#else - ch = value >> 16; -#endif - if (ch < 0) - ch = -ch; - if (ch > peak_r) - peak_r = ch; + if (s < 0) + s = -s; - addr += 4; + if ((uint32_t)s > peak_l) + peak_l = s; + + s = p[1]; + + if (s < 0) + s = -s; + + if ((uint32_t)s > peak_r) + peak_r = s; + + p += 4 * 2; /* Every 4th sample, interleaved */ } - while (addr < end); + while (p < pend); - peaks[0] = peak_l; - peaks[1] = peak_r; + peaks->left = peak_l; + peaks->right = peak_r; } void pcm_do_peak_calculation(struct pcm_peaks *peaks, bool active, @@ -177,7 +170,7 @@ void pcm_do_peak_calculation(struct pcm_peaks *peaks, bool active, else if (period > HZ/5) period = HZ/5; - peaks->period = (3*peaks->period + period) >> 2; + peaks->period = (3*peaks->period + period) / 4; peaks->tick = tick; if (active) @@ -186,29 +179,32 @@ void pcm_do_peak_calculation(struct pcm_peaks *peaks, bool active, count = MIN(framecount, count); if (count > 0) - pcm_peak_peeker((int32_t *)addr, count, peaks->val); + pcm_peak_peeker(addr, count, peaks); /* else keep previous peak values */ } else { /* peaks are zero */ - peaks->val[0] = peaks->val[1] = 0; + peaks->left = peaks->right = 0; } } void pcm_calculate_peaks(int *left, int *right) { + /* peak data for the global peak values - i.e. what the final output is */ + static struct pcm_peaks peaks; + int count; const void *addr = pcm_play_dma_get_peak_buffer(&count); - pcm_do_peak_calculation(&global_peaks, pcm_playing && !pcm_paused, + pcm_do_peak_calculation(&peaks, pcm_playing && !pcm_paused, addr, count); if (left) - *left = global_peaks.val[0]; + *left = peaks.left; if (right) - *right = global_peaks.val[1]; + *right = peaks.right; } const void* pcm_get_peak_buffer(int * count) @@ -471,20 +467,20 @@ static void pcm_recording_stopped(void) */ void pcm_calculate_rec_peaks(int *left, int *right) { - static uint16_t peaks[2]; + static struct pcm_peaks peaks; if (pcm_recording) { - const void *peak_addr = pcm_rec_peak_addr; - const void *addr = pcm_rec_dma_get_peak_buffer(); + const int16_t *peak_addr = pcm_rec_peak_addr; + const int16_t *addr = pcm_rec_dma_get_peak_buffer(); if (addr != NULL) { - int count = (int)(((intptr_t)addr - (intptr_t)peak_addr) >> 2); + int count = (addr - peak_addr) / 2; /* Interleaved L+R */ if (count > 0) { - pcm_peak_peeker((int32_t *)peak_addr, count, peaks); + pcm_peak_peeker(peak_addr, count, &peaks); if (peak_addr == pcm_rec_peak_addr) pcm_rec_peak_addr = addr; @@ -494,15 +490,15 @@ void pcm_calculate_rec_peaks(int *left, int *right) } else { - peaks[0] = peaks[1] = 0; + peaks.left = peaks.right = 0; } if (left) - *left = peaks[0]; + *left = peaks.left; if (right) - *right = peaks[1]; -} /* pcm_calculate_rec_peaks */ + *right = peaks.right; +} bool pcm_is_recording(void) { diff --git a/firmware/pcm_mixer.c b/firmware/pcm_mixer.c index 85d31ade70..cddff431ec 100644 --- a/firmware/pcm_mixer.c +++ b/firmware/pcm_mixer.c @@ -60,9 +60,6 @@ static size_t next_size = 0; /* Size of buffer to play next time */ /* Descriptors for all available channels */ static struct mixer_channel channels[PCM_MIXER_NUM_CHANNELS] IBSS_ATTR; -/* History for channel peaks */ -static struct pcm_peaks channel_peaks[PCM_MIXER_NUM_CHANNELS]; - /* Packed pointer array of all playing (active) channels in "channels" array */ static struct mixer_channel * active_channels[PCM_MIXER_NUM_CHANNELS+1] IBSS_ATTR; @@ -389,21 +386,14 @@ const void * mixer_channel_get_buffer(enum pcm_mixer_channel channel, int *count /* Calculate peak values for channel */ void mixer_channel_calculate_peaks(enum pcm_mixer_channel channel, - int *left, int *right) + struct pcm_peaks *peaks) { - struct mixer_channel *chan = &channels[channel]; - struct pcm_peaks *peaks = &channel_peaks[channel]; int count; const void *addr = mixer_channel_get_buffer(channel, &count); - pcm_do_peak_calculation(peaks, chan->status == CHANNEL_PLAYING, + pcm_do_peak_calculation(peaks, + channels[channel].status == CHANNEL_PLAYING, addr, count); - - if (left) - *left = peaks->val[0]; - - if (right) - *right = peaks->val[1]; } /* Adjust channel pointer by a given offset to support movable buffers */