Skip to content

Conversation

@ranj063
Copy link
Contributor

@ranj063 ranj063 commented Jul 23, 2025

The test runs the specified number of pause/resume iterations on a combination of a playback PCM and a capture PCM looping through all available playback and capture PCM devices available in the SOF audio card. It skips the deep buffer playback PCMs for the time being.

The test runs the specified number of pause/resume iterations on a
combination of a playback PCM and a capture PCM looping through all
available playback and capture PCM devices available in the SOF audio
card. It skips the deep buffer playback PCMs for the time being.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063 ranj063 requested review from a team, golowanow, lgirdwood and marc-hb as code owners July 23, 2025 20:11
@ranj063 ranj063 requested review from kv2019i and lyakh and removed request for marc-hb July 23, 2025 20:11
Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming it's the same feature, there is already a pause/resume test in shell script and expect, see c809fff. I wonder why this new one is needed.

I could be wrong but there is currently no support to compile C code in sof-test before deployment.

No harm in adding "manual" test code of course.

@lyakh
Copy link
Contributor

lyakh commented Jul 24, 2025

@ranj063 do you plan to add building and running the test to this PR? I think it would be important to see its results in the same PR

Copy link
Contributor

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial comments inline. I wonder if some existing tool could be used as well -- could be easier to integrate to sof-test runs in CI (versus starting to build C tools and deploy them to DUTs).


// if card name doesnt contain "sof", skip it
if (strstr(snd_ctl_card_info_get_name(card_info), "sof") == NULL) {
snd_ctl_close(ctl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not so obvious for the user. We could have SOF based cards that doen't have a "sof" prefix. A print to the user would be helpful.

@@ -0,0 +1,386 @@
#include <stdio.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some license needs to be attached.

}
// calculate wait time based on the period size and sample rate

usleep((playback_framesize / 48000) * 1000000); // Convert to microseconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixed to 48000 now, would be good to use a define this as many places in code depend on this.

cleanup_pcm_handles();
break;
}
usleep((capture_framesize / 48000) * 1000000); // Convert to microseconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not good, there should be a call on pcm_avail to block until there is enough data to read. This can get out of sync (CPU versus codec/DSP clock). Same for playback.

snd_pcm_hw_params_set_format(playback_handle, playback_params_1, SND_PCM_FORMAT_S16_LE);
snd_pcm_hw_params_set_channels(playback_handle, playback_params_1, 2);
snd_pcm_hw_params_set_rate(playback_handle, playback_params_1, 48000, 0);
playback_framesize = BUFFER_SIZE / (2 * 4); // 2 channels, 4 bytes per sample
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be separeate defines (channel count and sample format).

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be wrong but there is currently no support to compile C code in sof-test before deployment.

I wonder if some existing tool could be used as well -- could be easier to integrate to sof-test runs in CI (versus starting to build C tools and deploy them to DUTs).

Actually, doesn't this new test belong to https://github.com/alsa-project rather than sof-test? Please correct me but the only SOF-specific bit in this PR seems to be where the test ignores non-SOF cards (why?) Which cards to test looks like it should be a command line argument, not hard-coded.

Hosting this test in ALSA would solve the deployment problem since ALSA already needs a C compiler for deployment.

cc: @golowanow , @cgturner1

Now you can argue that a lot of sof-test is not that SOF-specific either and could/should be moved to https://github.com/alsa-project too. True but there are important differences:

  1. sof-test/test-case/ code is not brand new; it's much more difficult to move stuff that already exists
  2. it's shell and Python and not C code and does not have a C deployment issue. Not sure ALSA maintainers would even accept higher level languages much.
  3. it would be a challenge to disentangle the non-SOF specific bits while maintaining some stable interface with ALSA. Making sure the combinations of sof-test + ALSA versions deployed stay compatible with each other would be "fun".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants