Skip to content

module: dts: rework module to use sink/source api - #11212

Open
softwarecki wants to merge 1 commit into
thesofproject:mainfrom
softwarecki:p20-dts
Open

softwarecki wants to merge 1 commit into
thesofproject:mainfrom
softwarecki:p20-dts

Conversation

@softwarecki

Copy link
Copy Markdown
Collaborator

Rework the dts module to only use the sink/source api to prepare sof for the full transition to pipeline 2.0.

Rework the dts module to only use the sink/source api to prepare sof for
the full transition to pipeline 2.0.

Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The updated sink/source processing path has confirmed correctness issues (missing validation and non-standard pointer arithmetic) that can cause crashes or build failures.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR refactors the DTS codec module to use the module adapter sink/source APIs (process, source_get_*, sink_get_*) instead of the deprecated raw-data processing interface, as part of the transition toward pipeline 2.0.

Changes:

  • Switch DTS module processing from .process_raw_data to .process and update buffer parameter discovery to use struct sof_source.
  • Update runtime processing to pull input via source_get_data() and push output via sink_get_buffer()/sink_commit_buffer().
  • Tighten logging strings in DTS codec memory allocation and processing paths.
File summaries
File Description
src/audio/codec/dts/dts.c Migrates DTS codec to sink/source API usage for prepare and process paths.
Review details

Suppressed comments (3)

src/audio/codec/dts/dts.c:186

  • dts_codec_prepare() validates only num_of_sources but then assumes sources[0] is valid; it also ignores sinks even though the module is 1:1. Validate sources/sinks arrays and counts to avoid NULL deref later and to avoid unused-parameter warnings.
	if (num_of_sources < 1) {
		comp_err(dev, "invalid number of sources %d", num_of_sources);
		return -EINVAL;
	}

src/audio/codec/dts/dts.c:252

  • dts_codec_process() indexes sources[0]/sinks[0] without validating counts/pointers, and it only checks for input availability. If the sink has insufficient free space, the module may still consume/process input and then fail when acquiring the sink buffer. Validate counts/pointers and also gate processing on available sink free space.
	comp_dbg(dev, "start");

	/* Proceed only if we have enough data to fill the module buffer completely */
	if (source_get_data_available(sources[0]) < codec->mpd.in_buff_size) {
		comp_dbg(dev, "not enough data to process");

src/audio/codec/dts/dts.c:294

  • Pointer arithmetic on void * (snk_buf_start + buf_size) is not valid in standard C and can fail to compile with stricter toolchains. Cast to a byte pointer when computing the buffer end for cir_buf_bytes_without_wrap().
	size_to_wrap = cir_buf_bytes_without_wrap(snk_ptr, snk_buf_start + buf_size);
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/audio/codec/dts/dts.c
return ret;

comp_dbg(dev, "start");
size_to_wrap = cir_buf_bytes_without_wrap(src_ptr, src_buf_start + buf_size);
Comment thread src/audio/codec/dts/dts.c
Comment on lines 81 to +85

if (!source)
return -EINVAL;

stream = &source->stream;
buffer_fmt = audio_stream_get_buffer_fmt(stream);
frame_fmt = audio_stream_get_frm_fmt(stream);
rate = audio_stream_get_rate(stream);
channels = audio_stream_get_channels(stream);
buffer_fmt = source_get_buffer_fmt(source);
frame_fmt = source_get_frm_fmt(source);
rate = source_get_rate(source);
channels = source_get_channels(source);
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.

2 participants