Skip to content

rtnr: passthrough audio by stream copy - #5838

Merged
lgirdwood merged 1 commit into
thesofproject:mainfrom
MingJenTai:main_rtnr_bypass_stream
Jun 8, 2022
Merged

lgirdwood merged 1 commit into
thesofproject:mainfrom
MingJenTai:main_rtnr_bypass_stream

Conversation

@MingJenTai

Copy link
Copy Markdown
Contributor

This PR passthrough multi channels audio from microphone using stream copy outside RTNR, instead of using the bypass function provided by RTNR.

Signed-off-by: Ming Jen Tai mingjen_tai@realtek.com

@johnylin76

Copy link
Copy Markdown
Contributor

Thanks for the PR.
I am thinking that we could align the implementation to the one used on Redrix/Gimble.
Please see the attached patch.
The main difference is that cd->process_enable is checked under rtnr_copy() to choose the path (process or bypass), rather than making a new func rtnr_passthrough(). It would be more straightforward.

0001-RTNR-Decouple-switch-enable-control-from-params-blob.zip

@MingJenTai
MingJenTai force-pushed the main_rtnr_bypass_stream branch from 8dc5e01 to c396c23 Compare May 24, 2022 05:50
This commit passthrough multi channels audio from microphone using
stream copy outside RTNR, instead of using the bypass function provided
by RTNR.

Signed-off-by: Ming Jen Tai <mingjen_tai@realtek.com>
@MingJenTai
MingJenTai force-pushed the main_rtnr_bypass_stream branch from c396c23 to e914a37 Compare May 24, 2022 05:52
@MingJenTai

Copy link
Copy Markdown
Contributor Author

Thanks for the PR. I am thinking that we could align the implementation to the one used on Redrix/Gimble. Please see the attached patch. The main difference is that cd->process_enable is checked under rtnr_copy() to choose the path (process or bypass), rather than making a new func rtnr_passthrough(). It would be more straightforward.

0001-RTNR-Decouple-switch-enable-control-from-params-blob.zip

Force-pushed a commit to align the implementation.

@lgirdwood lgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@johnylin76 LGTM, but pls check it's aligned to Gimble etc.

@lgirdwood

Copy link
Copy Markdown
Member

@johnylin76 @cujomalainey good for you ?

Comment thread src/audio/rtnr/rtnr.c

buffer_stream_writeback(sink, cl.sink_bytes);
comp_update_buffer_consume(source, cl.source_bytes);
comp_update_buffer_produce(sink, cl.sink_bytes);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you could do

if (!cd->process_enable) {
	...
	return 0;
}

in the beginning and then leave the rest as it was or with very few changes, the delta would be much smaller then

@lgirdwood

Copy link
Copy Markdown
Member

Rerun the CI, looks like a lot of the DUTs were down or being used elsewhere.

@lgirdwood

Copy link
Copy Markdown
Member

SOFCI TEST

@lgirdwood
lgirdwood merged commit 605c666 into thesofproject:main Jun 8, 2022
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.

5 participants