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

AudioBufferSourceNode: inconsistencies in fast and slow track #527

Merged
merged 7 commits into from
Sep 18, 2024

Conversation

b-ma
Copy link
Collaborator

@b-ma b-ma commented Sep 13, 2024

Work in progress for #525

The failing tests are passing now but it introduced / revealed another issue, test_loop_hangs is now failing for another reason....

The semantics of loop start and end are not clear and they are thus not applied properly in edge cases I think, I need / will take some time to review all this properly

@orottier
Copy link
Owner

Right, I understand. Let me know if I can help somehow!

@b-ma
Copy link
Collaborator Author

b-ma commented Sep 16, 2024

Ok, I think the failing test was faulty actually, I renamed it to test_loop_out_of_bounds and added some comment, could you confirm you read it the same as me?

I left a bunch of todos I will try to tackle in the next days

Edit: also tested against wpt, no regression there

// Thus it violates the rule defined in
// https://webaudio.github.io/web-audio-api/#playback-AudioBufferSourceNode
// `loopStart >= 0 && loopEnd > 0 && loopStart < loopEnd`
// Hence the whole buffer should be looped
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, I agree. I did not put too much thought into the expected result earlier, just focused on fixing the indefinite loop. Thanks!

@orottier
Copy link
Owner

Alright nice going!

@b-ma b-ma marked this pull request as ready for review September 17, 2024 15:35
@b-ma
Copy link
Collaborator Author

b-ma commented Sep 17, 2024

Ok I think it's all good to me for this one

@orottier
Copy link
Owner

/bench

@orottier
Copy link
Owner

Benchmarks are currently failing #529
I was interested in the performance impact for the interpolation moving to f64. It requires some casting back and forth and increases the size of a 'hot' object. I will run the locally instead. Anyway, since this PR improves correctness, so the results of benchmarks are not too important

@orottier
Copy link
Owner

Okay maybe we don't have benchmarks for slow-track processing anyway.. oh well

@orottier orottier merged commit e78ad1a into orottier:main Sep 18, 2024
3 checks passed
@b-ma b-ma deleted the fix/audio-buffer-source-node-fast-track branch September 18, 2024 14:39
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.

2 participants