Audio: Sound Dose: Add new component by singalsu · Pull Request #10058 · thesofproject/sof

singalsu · 2025-06-13T17:19:43Z

No description provided.

src/audio/sound_dose/tune/sof_sound_dose_ref.m

src/audio/sound_dose/tune/sof_sound_dose_time_domain_filters.m

tools/topology/topology2/include/bench/sound_dose_hda_route.conf

src/audio/sound_dose/sound_dose-generic.c

src/include/user/sound_dose.h

lgirdwood

LGTM, just some opens.

src/audio/sound_dose/sound_dose.c

src/audio/sound_dose/sound_dose_iir_48k.h

src/include/user/sound_dose.h

lgirdwood

LGTM, think this looks ready for review

src/audio/sound_dose/sound_dose-generic.c

src/audio/sound_dose/sound_dose-ipc4.c

src/audio/sound_dose/sound_dose.c

kv2019i

I see you went with "notifications with timestamp" instead of a compressed stream of data. I guess this works as well, doesn't take DMA resources, and we can always add a DMA option later one. Added one comment about the timestamp accuracy if this is put in the common header.

kv2019i · 2025-07-01T10:42:24Z

src/include/user/audio_feature.h

If this is a common feature header, I wonder if "ms" is accurate enough? Maybe even report just number of frames since start at let user-space/app convert. For sound does, this would be 1ms increments.

Number of frames depends on content sample rate, so I thought the time as relative from start is more generic. Not sure if sound dose user space part has connection to the part that decodes stream. But we could have both in the struct?

Looks microseconds would work too, wrap in 580000 years so it would be more generic if need for high accuracy.

lgirdwood

just minor comments, needs more documentation, I would add a small ReadMe.md describing the flow and linking to the Android docs.

lgirdwood · 2025-08-18T12:39:59Z

tools/topology/topology2/cavs-benchmark-hda.conf

Can the commit message describe how test topology a bit more, e.g. the pipeline used to test.

lgirdwood · 2025-08-18T12:41:27Z

src/audio/sound_dose/sound_dose-ipc4.c

since this is the main transport for data to the host, can you describe the flow in more detail.

I added comments text

src/include/user/sound_dose.h

Copilot

Pull Request Overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-18T15:55:39Z

src/audio/sound_dose/sound_dose.h

+#include <stdint.h>
+
+#define SOUND_DOSE_DEBUG	0


[nitpick] Consider using a Kconfig option instead of a hardcoded debug flag to allow runtime configuration of debug output without recompilation.

Suggested change

#include <stdint.h>

#define SOUND_DOSE_DEBUG 0

#include <stdint.h>

#include <sof/common.h>

#ifdef CONFIG_SND_SOC_SOF_SOUND_DOSE_DEBUG

#define SOUND_DOSE_DEBUG 1

#else

#define SOUND_DOSE_DEBUG 0

#endif

Kconfig is a recompile and I'd not like to expose it globally. I find it easier to set it from this header file than going through "ninja -C build-xxx menuconfig"

Copilot · 2025-09-18T15:55:40Z

src/audio/sound_dose/sound_dose.c

+		/* TODO: 96 kHz and 192 kHz with integer decimation factor. The A-weight is not
+		 * defined above 20 kHz, so high frequency energy is not needed. Also it will
+		 * help keep the module load reasonable.


This TODO comment should include a ticket number or timeline for when this feature will be implemented, as it affects the component's supported sample rates.

Suggested change

/* TODO: 96 kHz and 192 kHz with integer decimation factor. The A-weight is not

* defined above 20 kHz, so high frequency energy is not needed. Also it will

* help keep the module load reasonable.

/* TODO (TICKET-1234): 96 kHz and 192 kHz with integer decimation factor. The A-weight is not

* defined above 20 kHz, so high frequency energy is not needed. Also it will

* help keep the module load reasonable.

* See TICKET-1234 for progress and timeline.

I need to discuss this with my manager, it depends on overall priorities.

This is fine, keep the TODO.

Copilot · 2025-09-18T15:55:40Z

src/audio/sound_dose/sound_dose-generic.c

[nitpick] The magic numbers for Q format (30, 15, 15) should be defined as named constants to improve code readability and maintainability.

Copilot · 2025-09-18T15:55:40Z

src/audio/sound_dose/sound_dose-generic.c

[nitpick] Similar to the S16 case, the Q format constants (30, 31, 31) should be defined as named constants for better maintainability.

kv2019i

No blockers!

src/audio/sound_dose/Kconfig

lgirdwood · 2025-09-19T12:36:54Z

src/audio/sound_dose/README.md

+normal topology file. Or alternative use module options
+tplg_filename and tplg_path for module snd_sof.
+
+Get kcontrol_events from https://github.com/ujfalusi/kcontrol_events


@ujfalusi please upstream this test too in here in a follow up PR. Thanks !

lgirdwood · 2025-09-22T09:14:24Z

@lrudyX ping - looks like CI got stuck ?

singalsu · 2025-09-23T13:20:25Z

@lgirdwood No changes, just pushed a rebase to restart CI checks.

singalsu · 2025-09-24T08:36:08Z

@lrudyX The two required CI checks seem to have got stuck since 19 hours when I re-pushed. The previous failure seemed to be CI internal, not caused by these code changes.

singalsu · 2025-09-24T11:40:17Z

Now I added toml for wcl platform, not sure, hoping it was the issue with rimage in build.

singalsu · 2025-09-25T14:49:46Z

There's a fail in https://sof-ci.01.org/sof-pr-viewer/#/build/PR10058/build14749499 with MTL TestRtcAec. I wonder how this PR can impact it.

[    0.543563] <inf> ipc: ipc_cmd: rx  : 0x40001000|0x12010026
[    0.544001] <wrn> ipc: ipc4_get_drv: the provided UUID (b780a0a6-269f-466f-b477-23dfa05af758) can't be found!
[    0.545855] <wrn> llext: llext_link_plt: PLT: cannot find idx 14 name 
[    0.546508] <wrn> llext: llext_link_plt: PLT: cannot find idx 14 name 
[    0.557715] <err> llext: do_llext_load: Failed to link, ret -2

kv2019i · 2025-09-26T07:21:58Z

@singalsu This had the QB fail still, so I now merged #10236 as it starts to block other PRs. IOW rebase is needed for this one,

The exported header file is missing the include for stdint.h for the used uint32_t type. Without it some builds fail to warning. Also the copyright text is updated. This patch also adds static const to the array declaration. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Need to drop the TLV header (used for Linux kernel bytes control) from data to keep the existing format after the header was added to sof-ctl generated data header. This header is not passed to firmware, only the kernel that this usage of filter coefficients passes, is using it. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch adds script sof_sound_dose_time_domain_filters.m that exports IIR and FIR coefficients to approximate A-weight function. The current choice is IIR only for lower MCPS load. The script sof_sound_dose_blobs.m creates a few control blobs to test the sound_dose component. A simple script sof_sound_dose_ref.m to compute dBFS and MEL for a wav file is added to compare with firmware reported values. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

The added plot is useful especially with parametric IIR equalizer tuning to see achieved response error vs. target. A numerical mean(abs()) value of error is printed to help see filter parameters change impact. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

The build topologies are - sof-hda-benchmark-sound_dose32.tplg - sof-mtl-sdw-benchmark-sound_dose32-sdw0.tplg - sof-mtl-sdw-benchmark-sound_dose32-simplejack.tplg The controls are for example initialized to - sensitivity 100 dB, 0 dBFS equals 100 dBSPL (a worst case loudness) - volume 0 dB, assumes codec gain for headphones is set to max - gain 0 dB, user's music playback is not attenuated Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

kv2019i · 2025-09-29T15:16:13Z

app/boards/intel_adsp/Kconfig.defconfig

If the CI issue persists, @lgirdwood proposed to limit the feature to newer platforms. @singalsu You can remove this bit completely and leave default as not set. For all the newer Intel platforms (ace20 and newer), sound dose will get enabled as a module.

OTOH, I don't see how enabling the component on MTL could break the stub loadable modules...

This patch adds a new SOF component Sound Dose. The purpose is to calculate for audio playback MEL values (momentary sound exposure level) to provide to user space the data to compute the sound dose CSD as defined in EN 50332-3. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

lrudyX · 2025-09-30T11:27:05Z

Looks like the last change has significant impact on test result. Now we have PASS on internal CI.

singalsu force-pushed the sound_dose branch 6 times, most recently from f038d75 to 710ed6d Compare June 16, 2025 16:52

singalsu commented Jun 17, 2025

View reviewed changes

singalsu force-pushed the sound_dose branch from 710ed6d to df86acd Compare June 18, 2025 16:32

singalsu commented Jun 19, 2025

View reviewed changes

src/include/user/sound_dose.h Outdated Show resolved Hide resolved

lgirdwood reviewed Jun 19, 2025

View reviewed changes

src/audio/sound_dose/sound_dose.c Show resolved Hide resolved

src/audio/sound_dose/sound_dose_iir_48k.h Show resolved Hide resolved

src/include/user/sound_dose.h Outdated Show resolved Hide resolved

singalsu force-pushed the sound_dose branch 3 times, most recently from 6f6db17 to a608928 Compare June 24, 2025 09:33

lgirdwood reviewed Jun 26, 2025

View reviewed changes

singalsu changed the title ~~[very draft - don't review] Audio: Sound Dose: Add new component~~ Audio: Sound Dose: Add new component Jun 27, 2025

singalsu force-pushed the sound_dose branch from a608928 to 047f684 Compare June 27, 2025 19:26

lyakh reviewed Jun 30, 2025

View reviewed changes

kv2019i reviewed Jul 1, 2025

View reviewed changes

singalsu force-pushed the sound_dose branch 2 times, most recently from c03beba to def60f6 Compare August 15, 2025 12:15

lgirdwood reviewed Aug 18, 2025

View reviewed changes

singalsu force-pushed the sound_dose branch from def60f6 to 33ea9ba Compare September 2, 2025 08:52

ujfalusi reviewed Sep 2, 2025

View reviewed changes

src/include/user/sound_dose.h Outdated Show resolved Hide resolved

singalsu force-pushed the sound_dose branch from 33ea9ba to 51873bf Compare September 4, 2025 16:33

singalsu force-pushed the sound_dose branch 5 times, most recently from 3aaf3a2 to 7f4e26d Compare September 17, 2025 13:25

singalsu marked this pull request as ready for review September 17, 2025 13:38

singalsu requested review from abonislawski, Copilot and kv2019i September 18, 2025 15:54

Copilot AI reviewed Sep 18, 2025

View reviewed changes

kv2019i approved these changes Sep 19, 2025

View reviewed changes

src/audio/sound_dose/Kconfig Outdated Show resolved Hide resolved

singalsu force-pushed the sound_dose branch from c702744 to f21c100 Compare September 19, 2025 10:05

lgirdwood approved these changes Sep 19, 2025

View reviewed changes

singalsu force-pushed the sound_dose branch 2 times, most recently from 85f5ff2 to f53a92d Compare September 19, 2025 14:10

abonislawski approved these changes Sep 22, 2025

View reviewed changes

singalsu force-pushed the sound_dose branch 2 times, most recently from 1452a41 to 81ec41b Compare September 23, 2025 13:19

singalsu force-pushed the sound_dose branch from 81ec41b to 1989a26 Compare September 24, 2025 11:11

singalsu added 5 commits September 29, 2025 11:39

singalsu force-pushed the sound_dose branch from 1989a26 to fdc91b9 Compare September 29, 2025 12:21

kv2019i reviewed Sep 29, 2025

View reviewed changes

singalsu force-pushed the sound_dose branch from fdc91b9 to b940776 Compare September 30, 2025 08:06

lgirdwood merged commit 951a27c into thesofproject:main Sep 30, 2025
27 of 47 checks passed

Conversation

singalsu commented Jun 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Sep 22, 2025

Uh oh!

singalsu commented Sep 23, 2025

Uh oh!

singalsu commented Sep 24, 2025

Uh oh!