roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.02k stars 204 forks source link

Implement lock-free fast_random function #596

Closed ForeverASilver closed 9 months ago

ForeverASilver commented 9 months ago

Commit for #426

gavv commented 9 months ago

Not sure if it's ready for review, but I checked new commits, the new version looks good!

I think this CI job failed because you have different version of clang-format. https://github.com/roc-streaming/roc-toolkit/actions/runs/6436815832/job/17480913363?pr=596

Which one do you use? I'm using 14.0.6. I think you can rollback changes in hashmap.h and check-formatting job will pass.

Other CI jobs failed because fast_random() is missing doxygen comment, please add it.

Please ping me when CI is fixed and PR is ready for review.

ForeverASilver commented 9 months ago

Weirdly enough formatting is still somehow an issue and i also have clang-format 14.0.6

gavv commented 9 months ago

I've pushed commit to this PR's branch that rolls back formatting: 671b26d8305af7d9d74cf8a854f9dd58d1bda84d

Here is how you can run the same version of clang-format that is used on CI:

docker run -t --rm -u "${UID}" -v "${PWD}:${PWD}" -w "${PWD}" rocstreaming/env-ubuntu scons -Q fmt

it should produce the same results that are expected on CI.

Ideally we need to investigate what's the problem and maybe adjust hashmap code to be unaffected by it, but we have ongoing issue that also touches hashmap (#579) and I'd like to avoid creating conflicts.

So for now, if you will work on something else in roc, you can use the docker command above. We can return to this problem later if needed.


Thanks for the patch! I'll merge it after CI passes.