Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tools: Topology2: Use rates instead of rate_min/max in benchmark #9218

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Tools: Topology2: Use rates instead of rate_min/max in benchmark #9218

merged 1 commit into from
Jun 13, 2024

Conversation

singalsu
Copy link
Collaborator

The rate_min/rate_max is obsolete, should use rates with list of rates to support. Without this change the playback of non-48 kHz content happens with 48 kHz rate with pitch shift effect. Aplay shows only a warning.

@singalsu
Tools: Topology2: Use rates instead of rate_min/max in benchmark
32b4b5a 
The rate_min/rate_max is obsolete, should use rates with
list of rates to support. Without this change the playback
of non-48 kHz content happens with 48 kHz rate with pitch
shift effect. Aplay shows only a warning.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
@singalsu singalsu mentioned this pull request Jun 11, 2024
Merged
@singalsu singalsu marked this pull request as ready for review June 11, 2024 10:11
@singalsu
Copy link
Collaborator Author

@ujfalusi Thanks for help, this change fixed the issue!

gbernatxintel
gbernatxintel approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@gbernatxintel gbernatxintel left a comment

Choose a reason for hiding this comment

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

In my opinion, the change is correct, but isn't there an error in the line above?
The format should not be saved
formats “S32_LE,S24_LE,S16_LE”

@singalsu
Copy link
Collaborator Author

In my opinion, the change is correct, but isn't there an error in the line above? The format should not be saved formats “S32_LE,S24_LE,S16_LE”

I didn't know. Is there a difference in using " " or ' ' or something else?

@lgirdwood
Copy link
Member

In my opinion, the change is correct, but isn't there an error in the line above? The format should not be saved formats “S32_LE,S24_LE,S16_LE”

I didn't know. Is there a difference in using " " or ' ' or something else?

IIUC the topology compiler will stringify if its missing ""

@lgirdwood
Copy link
Member

@singalsu I assume we need for v2.10 ?

@singalsu
Copy link
Collaborator Author

@singalsu I assume we need for v2.10 ?

None of these development topologies ends up to binary FW release, so I don't think it's needed. If there's source tarball for release, then it would make sense. How is it?

@lgirdwood
Copy link
Member

@singalsu I assume we need for v2.10 ?

None of these development topologies ends up to binary FW release, so I don't think it's needed. If there's source tarball for release, then it would make sense. How is it?

ok, not needed.

@kv2019i kv2019i merged commit 1470e6c into thesofproject:main Jun 13, 2024
39 of 46 checks passed
@singalsu singalsu deleted the tplg2_fix_benchmark_srcpcm_rates branch August 21, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgirdwood lgirdwood lgirdwood approved these changes

@ujfalusi ujfalusi ujfalusi approved these changes

@gbernatxintel gbernatxintel gbernatxintel approved these changes

@ranj063 ranj063 Awaiting requested review from ranj063 ranj063 is a code owner

@jsarha jsarha Awaiting requested review from jsarha jsarha is a code owner

@lyakh lyakh Awaiting requested review from lyakh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@singalsu @lgirdwood @ujfalusi @gbernatxintel @kv2019i