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

New "speexdec" resampler #595

Merged
merged 4 commits into from
Oct 9, 2023
Merged

New "speexdec" resampler #595

merged 4 commits into from
Oct 9, 2023

Conversation

gavv
Copy link
Member

@gavv gavv commented Oct 5, 2023

PR contains 3 commits:

  • relax requirements in resampler tests: use more smooth input, and 99% percentile for input & output comparisons
  • implement DecimationResampler (decorator for IResampler that add decimation for dynamic part of scaling factor, a.k.a. multiplier), and register "speexdec" backend that combines DecimationResampler + SpeexResampler
  • improve resampler tests: cleanup, cover multiple rates and profiles, add "scaling_trend" test

@gavv gavv requested a review from baranovmv October 5, 2023 17:50
@gavv gavv added the work in progress Pull request is still in progress and changing label Oct 5, 2023
@gavv gavv added ready for review Pull request can be reviewed and removed work in progress Pull request is still in progress and changing labels Oct 5, 2023
baranovmv
baranovmv requested changes Oct 8, 2023
View reviewed changes
src/internal_modules/roc_audio/decimation_resampler.cpp Outdated
// limit by output size, input size, and accumulator
const size_t copy_size = std::min(
out_size - out_pos,
std::min(in_size_ - in_pos_, (size_t)ceilf(out_acc_ / num_ch_) * num_ch_));
Copy link
Member

Choose a reason for hiding this comment

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

As I understand the code now, the 3rd branch in here should not evaluate ever, as 197-202 lines make it impossible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, this is an artifact from an older version. Fixed.

@gavv gavv requested a review from baranovmv October 9, 2023 11:06
@gavv
Copy link
Member Author

gavv commented Oct 9, 2023

Update: added ROC_RESAMPLER_BACKEND_SPEEXDEC to C API

gavv added 4 commits October 9, 2023 21:05
@gavv
Relax resampler tests
448b584 
- slower sine wave
- 99% percentile for sample comparison
@gavv
Implement SpeexDec resampler backend
686a319 
- add DecimationResampler class
- add SpeexDec backend that combines DecimationResampler
  + SpeexResampler
@gavv
Improve resampler tests
bf8c600 
- cleanup
- cover more combinations of parameters
- add scaling_trend test
@gavv
API: Add ROC_RESAMPLER_BACKEND_SPEEXDEC
ed70a1f
@baranovmv baranovmv merged commit 5f7d579 into roc-streaming:develop Oct 9, 2023
31 checks passed
@gavv gavv deleted the resampler branch October 9, 2023 21:44
@gavv gavv removed the ready for review Pull request can be reviewed label Oct 9, 2023
@gavv gavv added this to the 0.3.0 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baranovmv baranovmv baranovmv approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@gavv @baranovmv