[U-Boot] [PATCH 0/4] sound: Correct sound output on sandbox

At present sandbox sound output is disabled due to a bug. This series fixes that bug, adds a small API adjustment and enables sandbox sound output again.
Simon Glass (4): sound: Correct data output in sound_create_square_wave() sound: Add sample rate as a parameter for square wave sound: sandbox: Use the correct frequency sandbox: Enable sound
arch/sandbox/cpu/sdl.c | 30 +++++++++--------------------- drivers/sound/sound-i2s.c | 3 ++- drivers/sound/sound.c | 8 +++----- include/sound.h | 4 +++- 4 files changed, 17 insertions(+), 28 deletions(-)

This function currently outputs twice as much data as it should and overwrites its buffer as a result. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/sound/sound.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/sound/sound.c b/drivers/sound/sound.c index 969408186fd..6c1eb4c19cc 100644 --- a/drivers/sound/sound.c +++ b/drivers/sound/sound.c @@ -25,12 +25,10 @@ void sound_create_square_wave(unsigned short *data, int size, uint32_t freq) for (i = 0; size && i < half; i++) { size -= 2; *data++ = amplitude; - *data++ = amplitude; } for (i = 0; size && i < period - half; i++) { size -= 2; *data++ = -amplitude; - *data++ = -amplitude; } } }

This function currently outputs twice as much data as it should and overwrites its buffer as a result. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/sound/sound.c | 2 -- 1 file changed, 2 deletions(-)
Applied to u-boot-dm/master, thanks!

At present this value is hard-coded in the function that generates a square wave. Since sample rates vary between different hardware, it makes more sense to have this as a parameter.
Update the function and its users.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/sdl.c | 2 +- drivers/sound/sound-i2s.c | 3 ++- drivers/sound/sound.c | 6 +++--- include/sound.h | 4 +++- 4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index adcb73826fe..36f1bf0c83b 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -322,7 +322,7 @@ int sandbox_sdl_sound_start(uint frequency) if (!sdl.audio_active) return -1; sdl.frequency = frequency; - sound_create_square_wave((unsigned short *)sdl.audio_data, + sound_create_square_wave(22050, (unsigned short *)sdl.audio_data, sdl.audio_size, frequency); sdl.audio_pos = 0; SDL_PauseAudio(0); diff --git a/drivers/sound/sound-i2s.c b/drivers/sound/sound-i2s.c index 9f09e3bf930..f0f0b79bc52 100644 --- a/drivers/sound/sound-i2s.c +++ b/drivers/sound/sound-i2s.c @@ -185,7 +185,8 @@ int sound_play(uint32_t msec, uint32_t frequency) return -1; }
- sound_create_square_wave((unsigned short *)data, + sound_create_square_wave(g_i2stx_pri.samplingrate, + (unsigned short *)data, data_size / sizeof(unsigned short), frequency);
diff --git a/drivers/sound/sound.c b/drivers/sound/sound.c index 6c1eb4c19cc..4f0ad0d8f0d 100644 --- a/drivers/sound/sound.c +++ b/drivers/sound/sound.c @@ -7,11 +7,11 @@ #include <common.h> #include <sound.h>
-void sound_create_square_wave(unsigned short *data, int size, uint32_t freq) +void sound_create_square_wave(uint sample_rate, unsigned short *data, int size, + uint freq) { - const int sample = 48000; const unsigned short amplitude = 16000; /* between 1 and 32767 */ - const int period = freq ? sample / freq : 0; + const int period = freq ? sample_rate / freq : 0; const int half = period / 2;
assert(freq); diff --git a/include/sound.h b/include/sound.h index 3269f2371c3..77bfe6a93b2 100644 --- a/include/sound.h +++ b/include/sound.h @@ -31,11 +31,13 @@ struct sound_codec_info { /* * Generates square wave sound data for 1 second * + * @param sample_rate Sample rate in Hz * @param data data buffer pointer * @param size size of the buffer * @param freq frequency of the wave */ -void sound_create_square_wave(unsigned short *data, int size, uint32_t freq); +void sound_create_square_wave(uint sample_rate, unsigned short *data, int size, + uint freq);
/* * Initialises audio sub system

At present this value is hard-coded in the function that generates a square wave. Since sample rates vary between different hardware, it makes more sense to have this as a parameter.
Update the function and its users.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/sdl.c | 2 +- drivers/sound/sound-i2s.c | 3 ++- drivers/sound/sound.c | 6 +++--- include/sound.h | 4 +++- 4 files changed, 9 insertions(+), 6 deletions(-)
Applied to u-boot-dm/master, thanks!

At present we request a particular frequency but we may not get the exact same frequency in response. So use the actual frequency for generation of the square wave. This ensures that the pitch remains accurate on all host machines.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/sdl.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index 36f1bf0c83b..c940a473d7c 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -9,6 +9,10 @@ #include <sound.h> #include <asm/state.h>
+enum { + SAMPLE_RATE = 22050, +}; + static struct sdl_info { SDL_Surface *screen; int width; @@ -18,6 +22,7 @@ static struct sdl_info { uint frequency; uint audio_pos; uint audio_size; + uint sample_rate; uint8_t *audio_data; bool audio_active; bool inited; @@ -283,7 +288,7 @@ int sandbox_sdl_sound_init(void) return 0;
/* Set the audio format */ - wanted.freq = 22050; + wanted.freq = SAMPLE_RATE; wanted.format = AUDIO_S16; wanted.channels = 1; /* 1 = mono, 2 = stereo */ wanted.samples = 1024; /* Good low-latency value for callback */ @@ -309,6 +314,7 @@ int sandbox_sdl_sound_init(void) goto err; } sdl.audio_active = true; + sdl.sample_rate = wanted.freq;
return 0;
@@ -322,7 +328,8 @@ int sandbox_sdl_sound_start(uint frequency) if (!sdl.audio_active) return -1; sdl.frequency = frequency; - sound_create_square_wave(22050, (unsigned short *)sdl.audio_data, + sound_create_square_wave(sdl.sample_rate, + (unsigned short *)sdl.audio_data, sdl.audio_size, frequency); sdl.audio_pos = 0; SDL_PauseAudio(0);

At present we request a particular frequency but we may not get the exact same frequency in response. So use the actual frequency for generation of the square wave. This ensures that the pitch remains accurate on all host machines.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/sdl.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
Applied to u-boot-dm/master, thanks!

Now that the buffer-overflow bug is fixed, we can enable sound on sandbox. Drop the code which exits early.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/sdl.c | 19 ------------------- 1 file changed, 19 deletions(-)
diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index c940a473d7c..c7a8d945492 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -268,25 +268,6 @@ int sandbox_sdl_sound_init(void) if (sdl.audio_active) return 0;
- /* - * At present all sandbox sounds crash. This is probably due to - * symbol name conflicts with U-Boot. We can remove the malloc() - * probles with: - * - * #define USE_DL_PREFIX - * - * and get this: - * - * Assertion 'e->pollfd->fd == e->fd' failed at pulse/mainloop.c:676, - * function dispatch_pollfds(). Aborting. - * - * The right solution is probably to make U-Boot's names private or - * link os.c and sdl.c against their libraries before liking with - * U-Boot. TBD. For now sound is disabled. - */ - printf("(Warning: sandbox sound disabled)\n"); - return 0; - /* Set the audio format */ wanted.freq = SAMPLE_RATE; wanted.format = AUDIO_S16;

Now that the buffer-overflow bug is fixed, we can enable sound on sandbox. Drop the code which exits early.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/sdl.c | 19 ------------------- 1 file changed, 19 deletions(-)
Applied to u-boot-dm/master, thanks!
participants (2)
-
Simon Glass
-
sjg@google.com