Closed joshuatz closed 4 years ago
Thanks for the quick response and merge! Just FYI (or to anyone reading this), I had to hard refresh ("Empty Cache and Hard Reload") in my browser to get the change to show up on the live version, but once I did, everything seemed good.
Thanks again for all your hard work on this project!
Hi Joshua,
First, I just want to say thank you for all your hard work on this. This is seriously one of the most impressive web apps that I've ever seen, and I am constantly impressed by how much more intuitive and easy-to-use Beatmapper is in comparison with other map editor software. It is a pleasure to use, especially for novices like myself who are looking to create their first maps. I honestly can't say enough nice things about this app.
As for this PR, it should address multiple issues with the "playback rate" feature / slider (#91 & #94).
The main issue seemed to be that the
AudioSample
class method ofgetCurrentTime()
was returning the difference between the context current time (i.e. "now") and the sample start time, regardless of playbackRate. An easy fix would be to multiply this by theplaybackRate
(such as suggested here), but this makes a faulty assumption that the playbackRate has not changed throughout the song.My final fix was to basically to:
getCurrentTime()
, compute the "rate-adjusted" amount for the current segment, and add it to the already rate-adjusted (via startTime offset) previous segmentThis is not the only way this could have been done. If you would prefer a different approach, please let me know, and I can do my best to rewrite my solution.
This PR also fixes a very small bug related to playbackRate, called out in this comment. Basically, a fatal error is triggered if you try to change playbackRate before the song has loaded into memory.
Finally, as for tests, I started going down the path of adding a test to cover playbackRate timing computation, but immediately realized I was going to have issues with Jest, due to the lack of a mocked Web Audio API. If you really want tests added for this, I could look into either adding a Audio API mocking library to dependencies, or having tests run in a full browser, with something like Playwright or Puppeteer.
Thanks again for all your hard work on this! Please let me know if you have any feedback, required changes, etc.