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.

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

Multi threading #201

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Multi threading #201

wants to merge 18 commits into from

Conversation

ss3git
Copy link

@ss3git ss3git commented Sep 20, 2023

Experimental multi-thread implementation. (disabled by default with BUILD_MULTI_THREADING build option.)

Related issue: Parallelize resampling (with high delay) #176

compiled and tested on

FreeBSD clang version 14.0.5 (https://github.com/llvm/llvm-project.git llvmorg-14.0.5-0-gc12386ae247c)
Target: x86_64-unknown-freebsd13.2
Thread model: posix

compiled and tested on
--
FreeBSD clang version 14.0.5 (https://github.com/llvm/llvm-project.git llvmorg-14.0.5-0-gc12386ae247c)
Target: x86_64-unknown-freebsd13.2
Thread model: posix
--
@luzpaz
Copy link
Contributor

luzpaz commented Oct 28, 2023

Is there an ETA on this ?

@ss3git ss3git marked this pull request as draft November 14, 2023 11:25
…uffer for each thread seems to be larger in fewer channel process cases.
prefetch coefft to cache memory
@evpobr
Copy link
Member

evpobr commented Nov 17, 2023

@SoapGentoo , @arthurt , what do you think? I guess we can add it when it's ready.

@@ -43,6 +43,16 @@ add_library(samplerate
# ALIAS to use if libsamplerate is included from other project with add_subdirectory()
add_library(SampleRate::samplerate ALIAS samplerate)

if(MULTI_THREADING)
if(WIN32)
Copy link
Member

Choose a reason for hiding this comment

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

I guess we need to check MSVC variable. MinGW compiler probably will fail.

Copy link
Author

Choose a reason for hiding this comment

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

commit: 7348582

@ss3git ss3git marked this pull request as ready for review November 17, 2023 15:24
@ss3git
Copy link
Author

ss3git commented Nov 17, 2023

Hi. @evpobr

Sorry for that I did some commits after you had reviewed the code once.
The change may boost the performance when an integer equivalant src_ratio is in use.
I hope it works ok since I quickly merged it from my local branch (which supports S32 interface; if you are also interested: https://github.com/ss3git/libsamplerate/tree/S32Interface).

I have no more changes to this branch planned, so I'm leaving this up to you (or, maintainers) unless I find bugs.

One thing I'm aware of is that if you are actually releasing it, adding converter types like "SRC_SINC_BEST_QUALITY_MT" may be an idea to have both ST and MT versions switchable in one library.

… to the single-thread version.

note: the (single-thread) implementation may have some underlying bug here because the number of output frames is seemingly
inconsistent depending on the combinations of src_ratio, input frames, and number of channels.
The fix should be out of scope for this PR (MultiThreading).
@luzpaz
Copy link
Contributor

luzpaz commented Feb 10, 2024

Just wondering what's left here ?

@ss3git
Copy link
Author

ss3git commented Feb 11, 2024

Hello @luzpaz.

This PR works well enough at least on my machine.
Maybe waiting for more testers other than me?

@evpobr
Copy link
Member

evpobr commented Feb 26, 2024

@ss3git , are the results bitwise identical to the reference implementation?

@ss3git
Copy link
Author

ss3git commented Feb 26, 2024

Hi, @evpobr .

I've already forgotten a lot of the details though, the core calculation part is intended to be bitwise identical to the reference implementation (if you mean the single-thread version).

Since the single-thread implementation seems to have input/output processing variance depending on the number of channels processed (e.g. #206), I am not hundred percent sure if the outputs "always" match.

@evpobr
Copy link
Member

evpobr commented May 11, 2024

One thing I'm aware of is that if you are actually releasing it, adding converter types like "SRC_SINC_BEST_QUALITY_MT" may be an idea to have both ST and MT versions switchable in one library.

Good idea.

@luzpaz
Copy link
Contributor

luzpaz commented Jul 18, 2024

Sorry to bump, just wondering if there has been any more progress ?

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.

3 participants