[RFC 1/1] sound: allow waveform selection

* Allow the sound command to select the sine or the square waveform. * Allow to play multiple tones with one command. * Adjust documentation. * Adjust unit test.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- This would be the alternative to [v2,6/7] sound: add CONFIG_SOUND_SINE symbol For testing with the sandbox remove this line
arch/sandbox/dts/test.dts:969 sandbox,silent; /* Don't emit sounds while testing */
run the sand box with './u-boot -T' and issue the following commands
sound play sound play -s sound play -s 600 500 -q sound play -s 500 1047 500 880 500 0 500 1047 500 880 500 0 500 784 500 698 500 784 1000 698
Listening to the output demonstrates why patch 7/7 is needed. --- arch/sandbox/include/asm/test.h | 7 ++++ cmd/sound.c | 60 ++++++++++++++++++++++++++------- doc/usage/cmd/sound.rst | 28 ++++++++++++++- drivers/sound/sandbox.c | 7 ++++ drivers/sound/sound-uclass.c | 19 +++++++++-- include/sound.h | 21 +++++++++--- test/dm/sound.c | 45 ++++++++++++++++--------- 7 files changed, 151 insertions(+), 36 deletions(-)
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h index 568738c16d..199a38f57e 100644 --- a/arch/sandbox/include/asm/test.h +++ b/arch/sandbox/include/asm/test.h @@ -188,6 +188,13 @@ int sandbox_get_setup_called(struct udevice *dev); */ int sandbox_get_sound_active(struct udevice *dev);
+/** + * sandbox_reset_sound_count() - reset sound data count and sum + * + * @dev: device to reset + */ +void sandbox_reset_sound_count(struct udevice *dev); + /** * sandbox_get_sound_count() - Read back the count of the sound data so far * diff --git a/cmd/sound.c b/cmd/sound.c index 20ac3f758e..d1724e0b7c 100644 --- a/cmd/sound.c +++ b/cmd/sound.c @@ -39,26 +39,56 @@ static int do_play(struct cmd_tbl *cmdtp, int flag, int argc, int ret = 0; int msec = 1000; int freq = 400; - - if (argc > 1) - msec = dectoul(argv[1], NULL); - if (argc > 2) - freq = dectoul(argv[2], NULL); + enum sound_waveform waveform = SOUND_SQUARE; + bool first = true;
ret = uclass_first_device_err(UCLASS_SOUND, &dev); - if (!ret) - ret = sound_beep(dev, msec, freq); - if (ret) { - printf("Sound device failed to play (err=%d)\n", ret); - return CMD_RET_FAILURE; + if (ret) + goto err; + + --argc; + ++argv; + while (argc || first) { + first = false; + if (argc && *argv[0] == '-') { + switch (argv[0][1]) { + case 'q': + waveform = SOUND_SQUARE; + break; + case 's': + waveform = SOUND_SINE; + break; + default: + return CMD_RET_USAGE; + } + --argc; + ++argv; + } + if (argc && *argv[0] != '-') { + msec = dectoul(argv[0], NULL); + --argc; + ++argv; + } + if (argc && *argv[0] != '-') { + freq = dectoul(argv[0], NULL); + --argc; + ++argv; + } + ret = sound_beep(dev, msec, freq, waveform); + if (ret) + goto err; }
return 0; + +err: + printf("Sound device failed to play (err=%d)\n", ret); + return CMD_RET_FAILURE; }
static struct cmd_tbl cmd_sound_sub[] = { U_BOOT_CMD_MKENT(init, 0, 1, do_init, "", ""), - U_BOOT_CMD_MKENT(play, 2, 1, do_play, "", ""), + U_BOOT_CMD_MKENT(play, INT_MAX, 1, do_play, "", ""), };
/* process sound command */ @@ -83,8 +113,12 @@ static int do_sound(struct cmd_tbl *cmdtp, int flag, int argc, }
U_BOOT_CMD( - sound, 4, 1, do_sound, + sound, INT_MAX, 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 [[[-q|-s] len [freq]] ...] - play a sound\n" + " -q - square waveform\n" + " -s - sine waveform\n" + " len - duration in ms\n" + " freq - frequency in Hz\n" ); diff --git a/doc/usage/cmd/sound.rst b/doc/usage/cmd/sound.rst index d3fac243b1..03e3a2de78 100644 --- a/doc/usage/cmd/sound.rst +++ b/doc/usage/cmd/sound.rst @@ -10,7 +10,7 @@ Synopsis ::
sound init - sound play [len [freq]] + sound play [[[-q|-s] len [freq]] ...]
Description ----------- @@ -24,12 +24,38 @@ sound play plays a square wave sound. It does not depend on previously calling *sound init*.
+-q + generate a square waveform (this is the default) + +-s + generate a sine waveform + len duration of the sound in ms, defaults to 1000 ms
freq frequency of the sound in Hz, defaults to 400 Hz
+Examples +-------- + +Square wave beep with 400 Hz for 1000 ms:: + + sound play + +Sine wave beep with 400 Hz for 1000 ms:: + + sound play -s + +Sine wave beep at 500 Hz for 600 ms followed by square wave beep at 500 Hz +for 600 ms:: + + sound play -s 600 500 -q + +Melody played with sine waveform:: + + sound play -s 500 1047 500 880 500 0 500 1047 500 880 500 0 500 784 500 698 500 784 1000 698 + Configuration -------------
diff --git a/drivers/sound/sandbox.c b/drivers/sound/sandbox.c index c6cbd81fdb..693611fcc8 100644 --- a/drivers/sound/sandbox.c +++ b/drivers/sound/sandbox.c @@ -69,6 +69,13 @@ int sandbox_get_sound_active(struct udevice *dev) return priv->active; }
+void sandbox_reset_sound_count(struct udevice *dev) { + struct sandbox_sound_priv *priv = dev_get_priv(dev); + + priv->count = 0; + priv->sum = 0; +} + int sandbox_get_sound_count(struct udevice *dev) { struct sandbox_sound_priv *priv = dev_get_priv(dev); diff --git a/drivers/sound/sound-uclass.c b/drivers/sound/sound-uclass.c index 2ffc4fc7c1..c459b86ef1 100644 --- a/drivers/sound/sound-uclass.c +++ b/drivers/sound/sound-uclass.c @@ -66,7 +66,8 @@ int sound_stop_beep(struct udevice *dev) return ops->stop_beep(dev); }
-int sound_beep(struct udevice *dev, int msecs, int frequency_hz) +int sound_beep(struct udevice *dev, int msecs, int frequency_hz, + enum sound_waveform waveform) { struct sound_uc_priv *uc_priv = dev_get_uclass_priv(dev); struct i2s_uc_priv *i2s_uc_priv; @@ -99,8 +100,20 @@ int sound_beep(struct udevice *dev, int msecs, int frequency_hz) return -ENOMEM; }
- sound_create_square_wave(i2s_uc_priv->samplingrate, data, data_size, - frequency_hz, i2s_uc_priv->channels); + switch (waveform) { + case SOUND_SINE: + sound_create_sine_wave(i2s_uc_priv->samplingrate, data, + data_size, frequency_hz, + i2s_uc_priv->channels); + break; + case SOUND_SQUARE: + sound_create_square_wave(i2s_uc_priv->samplingrate, data, + data_size, frequency_hz, + i2s_uc_priv->channels); + break; + default: + return -EINVAL; + }
ret = 0; while (msecs >= 1000) { diff --git a/include/sound.h b/include/sound.h index cf9c3e8fb7..be9bdc58bf 100644 --- a/include/sound.h +++ b/include/sound.h @@ -19,6 +19,17 @@ struct sound_codec_info { int i2c_dev_addr; };
+/** + * enum sound_waveform - sound waveform + * + * @SOUND_SQUARE: square waveform + * @SOUND_SINE: sine waveform + */ +enum sound_waveform { + SOUND_SQUARE, + SOUND_SINE, +}; + /** * struct sound_uc_priv - private uclass information about each sound device * @@ -125,12 +136,14 @@ int sound_setup(struct udevice *dev); /** * play() - Play a beep * - * @dev: Sound device - * @msecs: Duration of beep in milliseconds - * @frequency_hz: Frequency of the beep in Hertz + * @dev: sound device + * @msecs: duration of beep in milliseconds + * @frequency_hz: frequency of the beep in Hertz + * @waveform: waveform * Return: 0 if OK, -ve on error */ -int sound_beep(struct udevice *dev, int msecs, int frequency_hz); +int sound_beep(struct udevice *dev, int msecs, int frequency_hz, + enum sound_waveform waveform);
/** * sound_start_beep() - Start beeping diff --git a/test/dm/sound.c b/test/dm/sound.c index 15d545ab5a..cfa6306527 100644 --- a/test/dm/sound.c +++ b/test/dm/sound.c @@ -12,11 +12,19 @@ #include <test/test.h> #include <asm/test.h>
+ /* Basic test of the sound codec uclass */ static int dm_test_sound(struct unit_test_state *uts) { struct sound_uc_priv *uc_priv; struct udevice *dev; + struct test_data { + enum sound_waveform waveform; + int expected; + } test_data[2] = { + {SOUND_SQUARE, 4560}, + {SOUND_SINE, 3494}, + };
/* check probe success */ ut_assertok(uclass_first_device_err(UCLASS_SOUND, &dev)); @@ -25,21 +33,28 @@ static int dm_test_sound(struct unit_test_state *uts) ut_asserteq_str("i2s", uc_priv->i2s->name); 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)); + for (int i = 0; i < 2; ++i) { + int expected = test_data->expected; + + sandbox_reset_sound_count(dev); + ut_assertok(sound_beep(dev, 1, 100, test_data->waveform)); + ut_asserteq(48, sandbox_get_sound_count(dev)); + ut_asserteq(expected, sandbox_get_sound_sum(dev)); + ut_assertok(sound_beep(dev, 1, 100, test_data->waveform)); + ut_asserteq(96, sandbox_get_sound_count(dev)); + expected *= 2; + ut_asserteq(expected, sandbox_get_sound_sum(dev)); + ut_assertok(sound_beep(dev, 1, -100, test_data->waveform)); + ut_asserteq(144, sandbox_get_sound_count(dev)); + ut_asserteq(expected, sandbox_get_sound_sum(dev)); + ut_assertok(sound_beep(dev, 1, 0, test_data->waveform)); + ut_asserteq(192, sandbox_get_sound_count(dev)); + ut_asserteq(expected, sandbox_get_sound_sum(dev)); + ut_assertok(sound_beep(dev, 1, INT_MAX, test_data->waveform)); + ut_asserteq(240, sandbox_get_sound_count(dev)); + ut_asserteq(expected, sandbox_get_sound_sum(dev)); + } + ut_asserteq(false, sandbox_get_sound_active(dev));
return 0;

Hi Heinrich,
On Mon, 5 Dec 2022 at 13:38, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
- Allow the sound command to select the sine or the square waveform.
- Allow to play multiple tones with one command.
- Adjust documentation.
- Adjust unit test.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
This would be the alternative to [v2,6/7] sound: add CONFIG_SOUND_SINE symbol For testing with the sandbox remove this line
arch/sandbox/dts/test.dts:969 sandbox,silent; /* Don't emit sounds while testing */
run the sand box with './u-boot -T' and issue the following commands
sound play sound play -s sound play -s 600 500 -q sound play -s 500 1047 500 880 500 0 500 1047 500 880 500 0 500 784 500 698 500 784 1000 698
Listening to the output demonstrates why patch 7/7 is needed.
arch/sandbox/include/asm/test.h | 7 ++++ cmd/sound.c | 60 ++++++++++++++++++++++++++------- doc/usage/cmd/sound.rst | 28 ++++++++++++++- drivers/sound/sandbox.c | 7 ++++ drivers/sound/sound-uclass.c | 19 +++++++++-- include/sound.h | 21 +++++++++--- test/dm/sound.c | 45 ++++++++++++++++--------- 7 files changed, 151 insertions(+), 36 deletions(-)
This seems OK to me. Perhaps add a few run_command() tests as well?
Regards, Simon

On 12/6/22 00:55, Simon Glass wrote:
Hi Heinrich,
On Mon, 5 Dec 2022 at 13:38, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
- Allow the sound command to select the sine or the square waveform.
- Allow to play multiple tones with one command.
- Adjust documentation.
- Adjust unit test.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
This would be the alternative to [v2,6/7] sound: add CONFIG_SOUND_SINE symbol For testing with the sandbox remove this line
arch/sandbox/dts/test.dts:969 sandbox,silent; /* Don't emit sounds while testing */
run the sand box with './u-boot -T' and issue the following commands
sound play sound play -s sound play -s 600 500 -q sound play -s 500 1047 500 880 500 0 500 1047 500 880 500 0 500 784 500 698 500 784 1000 698
Listening to the output demonstrates why patch 7/7 is needed.
arch/sandbox/include/asm/test.h | 7 ++++ cmd/sound.c | 60 ++++++++++++++++++++++++++------- doc/usage/cmd/sound.rst | 28 ++++++++++++++- drivers/sound/sandbox.c | 7 ++++ drivers/sound/sound-uclass.c | 19 +++++++++-- include/sound.h | 21 +++++++++--- test/dm/sound.c | 45 ++++++++++++++++--------- 7 files changed, 151 insertions(+), 36 deletions(-)
This seems OK to me. Perhaps add a few run_command() tests as well?
Hello Simon,
thanks for reviewing.
I have created a pull request for the bug fixes to go into 2023.01. As it is late in the cycle I don't want to introduce bigger changes.
On a system with multiple sound devices playback on the sandbox hangs in sandbox_sdl_sound_stop() waiting for sdl.stopping. This needs further investigation.
Best regards
Heinrich
Regards, Simon
participants (2)
-
Heinrich Schuchardt
-
Simon Glass