BPM tracking improvement#1657
Conversation
I did not remove the old code to minimize changes, you can beat me up now
There was a problem hiding this comment.
Code Review
This pull request integrates the realtime-bpm-analyzer library to enable real-time BPM detection and metronome synchronization. Key updates include modifying the audio worklet to output high-fidelity audio and implementing a metronome that adjusts its interval based on detected BPM and audio energy. Review feedback highlights the need to resolve variable shadowing, properly configure the BPM analyzer instance, and ensure the frequency buffer is updated before energy calculations. Additionally, it is suggested to remove redundant memory allocations and debug workarounds.
| channels: options.processorOptions?.channels || 1, | ||
| converterType: options.processorOptions?.converterType || ConverterType.SRC_SINC_MEDIUM_QUALITY, | ||
| bufferSize: options.processorOptions?.bufferSize || 4096, | ||
| bufferSize: options.processorOptions?.bufferSize || 16384, |
There was a problem hiding this comment.
There was a problem hiding this comment.
the increase in necessary due to the increase in sample rate, which in turn allows for a broader spectrum to be recognized by the BPM detector
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5c7dcac88
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
~~I blame Gemini~~
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01143cd987
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cd336d2de
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Refactor audio data analysis method and remove async.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31ff4ff0fd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6f18558bc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| '@better-auth/drizzle-adapter': | ||
| specifier: ^1.6.2 | ||
| version: 1.6.2(@better-auth/core@1.6.2(@better-auth/utils@0.4.0)(@better-fetch/fetch@1.1.21)(@opentelemetry/api@1.9.1)(better-call@1.3.5(zod@4.3.6))(jose@6.2.2)(kysely@0.28.16)(nanostores@1.2.0))(@better-auth/utils@0.4.0)(drizzle-orm@0.45.2(@electric-sql/pglite@0.4.4)(@opentelemetry/api@1.9.1)(@prisma/client@5.22.0)(@types/pg@8.20.0)(better-sqlite3@12.8.0)(kysely@0.28.16)(pg@8.20.0)(postgres@3.4.9)) | ||
| version: 1.6.2(@better-auth/core@1.4.21(@better-auth/utils@0.3.0)(@better-fetch/fetch@1.1.21)(better-call@1.1.8(zod@4.3.6))(jose@6.2.2)(kysely@0.28.16)(nanostores@1.2.0))(@better-auth/utils@0.3.0)(drizzle-orm@0.45.2(@electric-sql/pglite@0.4.4)(@opentelemetry/api@1.9.1)(@prisma/client@5.22.0)(@types/pg@8.20.0)(better-sqlite3@12.8.0)(kysely@0.28.16)(pg@8.20.0)(postgres@3.4.9)) |
There was a problem hiding this comment.
Keep server lockfile on compatible better-auth peer set
This lockfile change rewires apps/server to @better-auth/drizzle-adapter@1.6.2 resolved with @better-auth/core@1.4.21 and @better-auth/utils@0.3.0, which is incompatible with that adapter’s declared peer range (@better-auth/core ^1.6.2, utils 0.4.0). Because pnpm-lock.yaml is authoritative for frozen installs, this can introduce peer conflicts and unstable auth behavior unrelated to the BPM feature.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not my job.
(I never touched those)
Thanks codex, again.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9de0bec9ef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Updated createBeatSyncDetector to be asynchronous and modified the start method to call syncMetronome.
…i-beat-sync into bpm-tracker-mod
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24a88bc2c0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1015724f9f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| worklet: analyserWorklet, | ||
| listeners: { | ||
| onBeat: e => emit('beat', e), | ||
| onBeat: () => {}, |
There was a problem hiding this comment.
Keep forwarding TempoRA beat callbacks
Changing onBeat to a no-op removes the existing beat signal path from startTemporaAnalyser, so beat listeners now depend entirely on the new realtime-BPM path producing lock messages. In scenarios where that path is delayed or fails (e.g., no compatible audioData messages, analyzer error), beat sync emits nothing and appears broken, whereas the previous implementation still delivered beat events.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fallback emits 40 BPM beat events
|
Do you have any evidence that this is better? |
|
Instead of trying to pick up each spike in the audio stream, this tries to get a BPM that would lock in. This should make it more reliable and pick up beats from music that does not have spikes on beat. |
|
Would you mind switch the implementation to have them co-exist for a while and let the user to judge which one is better? So
|
|
That would actually be a great idea, I'll try doing that. Should I close this PR and start a new one when I am ready, or should the PR be kept open to wait for the changes? |
|
Since there is no activity, I'll be closing this PR and open a new one when I finish implementing the choice. |
Description
An attempt in switching to a better BPM detector to improve BPM detection accuracy.
Linked Issues
N/A
Additional Context
First time commit, may make a lot of mistakes. Also I did not remove the old code to minimize changes...