From d0e8777beeb675b77d6b2bf679133106892eb8dd Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 4 Dec 2022 14:00:05 +0100 Subject: sound: avoid endless loop 'sound play 1 100000' results in an endless loop on the sandbox. If the frequency exceeds half the sampling rate, zero out the output buffer. Fixes: 511ed5fdd389 ("SOUND: SAMSUNG: Add I2S driver") Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- drivers/sound/sound.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/sound/sound.c b/drivers/sound/sound.c index 041dfdccfeb..c0fc50c99da 100644 --- a/drivers/sound/sound.c +++ b/drivers/sound/sound.c @@ -15,7 +15,10 @@ void sound_create_square_wave(uint sample_rate, unsigned short *data, int size, const int period = freq ? sample_rate / freq : 0; const int half = period / 2; - assert(freq); + if (!half) { + memset(data, 0, size); + return; + } /* Make sure we don't overflow our buffer */ if (size % 2) -- cgit v1.3.1 From 968eaaeaa7a8cf0956e19e9217af9e81bc7fb2eb Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 4 Dec 2022 17:11:41 +0100 Subject: test: test sandbox sound driver more rigorously Consider unexpected values for frequency: * negative frequency * zero frequency * frequency exceeding sampling frequency As in these cases the sum of the samples is zero also check the count of the samples. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- arch/sandbox/include/asm/test.h | 10 ++++++++++ drivers/sound/sandbox.c | 9 +++++++++ test/dm/sound.c | 11 +++++++++++ 3 files changed, 30 insertions(+) diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h index 0406085917f..568738c16d5 100644 --- a/arch/sandbox/include/asm/test.h +++ b/arch/sandbox/include/asm/test.h @@ -188,6 +188,16 @@ int sandbox_get_setup_called(struct udevice *dev); */ int sandbox_get_sound_active(struct udevice *dev); +/** + * sandbox_get_sound_count() - Read back the count of the sound data so far + * + * This data is provided to the sandbox driver by the sound play() method. + * + * @dev: Device to check + * Return: count of audio data + */ +int sandbox_get_sound_count(struct udevice *dev); + /** * sandbox_get_sound_sum() - Read back the sum of the sound data so far * diff --git a/drivers/sound/sandbox.c b/drivers/sound/sandbox.c index 4a2c87a84c6..c6cbd81fdbc 100644 --- a/drivers/sound/sandbox.c +++ b/drivers/sound/sandbox.c @@ -29,6 +29,7 @@ struct sandbox_i2s_priv { struct sandbox_sound_priv { int setup_called; /* Incremented when setup() method is called */ bool active; /* TX data is being sent */ + int count; /* Use to count the provided audio data */ int sum; /* Use to sum the provided audio data */ bool allow_beep; /* true to allow the start_beep() interface */ int frequency_hz; /* Beep frequency if active, else 0 */ @@ -68,6 +69,13 @@ int sandbox_get_sound_active(struct udevice *dev) return priv->active; } +int sandbox_get_sound_count(struct udevice *dev) +{ + struct sandbox_sound_priv *priv = dev_get_priv(dev); + + return priv->count; +} + int sandbox_get_sound_sum(struct udevice *dev) { struct sandbox_sound_priv *priv = dev_get_priv(dev); @@ -168,6 +176,7 @@ static int sandbox_sound_play(struct udevice *dev, void *data, uint data_size) for (i = 0; i < data_size; i++) priv->sum += ((uint8_t *)data)[i]; + priv->count += data_size; return i2s_tx_data(uc_priv->i2s, data, data_size); } diff --git a/test/dm/sound.c b/test/dm/sound.c index b73f6ab1113..15d545ab5a3 100644 --- a/test/dm/sound.c +++ b/test/dm/sound.c @@ -26,8 +26,19 @@ static int dm_test_sound(struct unit_test_state *uts) ut_asserteq(0, sandbox_get_setup_called(dev)); ut_assertok(sound_beep(dev, 1, 100)); + ut_asserteq(48, sandbox_get_sound_count(dev)); ut_asserteq(4560, sandbox_get_sound_sum(dev)); ut_assertok(sound_beep(dev, 1, 100)); + ut_asserteq(96, sandbox_get_sound_count(dev)); + ut_asserteq(9120, sandbox_get_sound_sum(dev)); + ut_assertok(sound_beep(dev, 1, -100)); + ut_asserteq(144, sandbox_get_sound_count(dev)); + ut_asserteq(9120, sandbox_get_sound_sum(dev)); + ut_assertok(sound_beep(dev, 1, 0)); + ut_asserteq(192, sandbox_get_sound_count(dev)); + ut_asserteq(9120, sandbox_get_sound_sum(dev)); + ut_assertok(sound_beep(dev, 1, INT_MAX)); + ut_asserteq(240, sandbox_get_sound_count(dev)); ut_asserteq(9120, sandbox_get_sound_sum(dev)); ut_asserteq(false, sandbox_get_sound_active(dev)); -- cgit v1.3.1 From ff414fcc44b8b385600df54f5a7dc0719cec106f Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 4 Dec 2022 16:33:07 +0100 Subject: cmd: fix long text for sound command Make it clear that if only 1 parameter is provided this is the duration. The ISO symbol for hertz is Hz. Fixes: c0c88533fffd ("Sound: Add command for audio playback") Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- cmd/sound.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/sound.c b/cmd/sound.c index f82f2aa6708..20ac3f758e3 100644 --- a/cmd/sound.c +++ b/cmd/sound.c @@ -86,5 +86,5 @@ U_BOOT_CMD( sound, 4, 1, do_sound, "sound sub-system", "init - initialise the sound driver\n" - "sound play [len] [freq] - play a sound for len ms at freq hz\n" + "sound play [len [freq]] - play a sound for len ms at freq Hz\n" ); -- cgit v1.3.1 From 3f01307ced9af0c20b48217c2c58b63f824451c5 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 4 Dec 2022 16:16:12 +0100 Subject: doc: man-page for the sound command Provide a man-page for the sound command. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- doc/usage/cmd/sound.rst | 41 +++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 42 insertions(+) create mode 100644 doc/usage/cmd/sound.rst diff --git a/doc/usage/cmd/sound.rst b/doc/usage/cmd/sound.rst new file mode 100644 index 00000000000..d3fac243b13 --- /dev/null +++ b/doc/usage/cmd/sound.rst @@ -0,0 +1,41 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. Copyright 2022, Heinrich Schuchardt + +sound command +============= + +Synopsis +-------- + +:: + + sound init + sound play [len [freq]] + +Description +----------- + +The *sound* command is used to play a beep sound. + +sound init + initializes the sound driver. + +sound play + plays a square wave sound. It does not depend on previously calling + *sound init*. + +len + duration of the sound in ms, defaults to 1000 ms + +freq + frequency of the sound in Hz, defaults to 400 Hz + +Configuration +------------- + +The sound command is enabled by CONFIG_CMD_SOUND=y. + +Return value +------------ + +The return value $? is 0 (true) if the command succeeds, 1 (false) otherwise. diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 0bc82887e9e..bbd40a6e189 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -73,6 +73,7 @@ Shell commands cmd/scp03 cmd/setexpr cmd/size + cmd/sound cmd/temperature cmd/tftpput cmd/true -- cgit v1.3.1 From 304bc9f437df51b4d982fe25fd0988350c8f5fc9 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 4 Dec 2022 20:48:57 +0100 Subject: sandbox: fix sound driver In the callback function we have to use memcpy(). Otherwise we add the new samples on top of what is stored in the stream buffer. If we don't have enough data, zero out the rest of the stream buffer. Our sampling frequency is 48000. Let the batch size for the callback function be 960. If we play a multiple of 20 ms, this will always be a full batch. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- arch/sandbox/cpu/sdl.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index f4ca36b35c8..2c570ed8d16 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -441,7 +441,6 @@ void sandbox_sdl_fill_audio(void *udata, Uint8 *stream, int len) { struct buf_info *buf; int avail; - bool have_data = false; int i; for (i = 0; i < 2; i++) { @@ -453,10 +452,9 @@ void sandbox_sdl_fill_audio(void *udata, Uint8 *stream, int len) } if (avail > len) avail = len; - have_data = true; - SDL_MixAudio(stream, buf->data + buf->pos, avail, - SDL_MIX_MAXVOLUME); + memcpy(stream, buf->data + buf->pos, avail); + stream += avail; buf->pos += avail; len -= avail; @@ -466,7 +464,8 @@ void sandbox_sdl_fill_audio(void *udata, Uint8 *stream, int len) else break; } - sdl.stopping = !have_data; + memset(stream, 0, len); + sdl.stopping = !!len; } int sandbox_sdl_sound_init(int rate, int channels) @@ -484,7 +483,7 @@ int sandbox_sdl_sound_init(int rate, int channels) wanted.freq = rate; wanted.format = AUDIO_S16; wanted.channels = channels; - wanted.samples = 1024; /* Good low-latency value for callback */ + wanted.samples = 960; /* Good low-latency value for callback */ wanted.callback = sandbox_sdl_fill_audio; wanted.userdata = NULL; -- cgit v1.3.1