Skip to content

JzzMidiAccess has a bug that prevents it to read midi values from js object#105

Merged
atsushieno merged 3 commits into
atsushieno:mainfrom
e13mort:jzz-fix
Feb 25, 2026
Merged

JzzMidiAccess has a bug that prevents it to read midi values from js object#105
atsushieno merged 3 commits into
atsushieno:mainfrom
e13mort:jzz-fix

Conversation

@e13mort

@e13mort e13mort commented Feb 23, 2026

Copy link
Copy Markdown
Contributor
  • Manually read values from js object in a loop
  • use Date.now() function for timestamp calculation
  • update JzzMidi library to 1.8.6

@atsushieno

Copy link
Copy Markdown
Owner

Thanks for the PR!

I have a question about your timestamp changes:

    val timestampInNanoseconds = Date.now() * 1000

Doen't Date.now() hold time in milliseconds? It seems like we need 1000000 there.

@e13mort

e13mort commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Doen't Date.now() hold time in milliseconds? It seems like we need 1000000 there.

MDN says that it's in milliseconds https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/now

If that's true do we need multiplication by 1_000_000?

@atsushieno

Copy link
Copy Markdown
Owner

Yes, that's my point.

I'm actually not sure if Date is better than DOMHighResTimestamp. Date sounds compromising to me, but I have no strong preference. Thoughts?

@e13mort

e13mort commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

I see a few possible solutions here:

  • rollback to passing 0 value so it'd be more explicit way to specify that logic isn't implemented yet. easy, but further work is required
  • keep proposed Date solution. easy, seems like covers most of cases (IMHO), less suitable for precise timing logic, might be enough for current library state
  • implement DOMHighResTimestamp for maximum precision. not that easy because of relative nature of that API (but not that hard because of timeOrigin property). that logic (as well as the second option) leads to a question: should that callback return a specific date related value (with timezone etc) with midi event or the value should be interpreted just as "some millis that you can use only for delta calculations". That logic definitely should be consistent along other MidiAccess implementations and I don't have a related context here for now.

Personally I'd prefer the second option. The first one fits too, because my goal was to fix ByteArray bug at the first place. The third option is likely the right one, but it requires more research in order to achieve the consistent behaviour.

@atsushieno

Copy link
Copy Markdown
Owner

OK, thanks. I agree to your points. Can you update your changes so that the time unit matches to nanoseconds?

@e13mort

e13mort commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Great!

Can you update your changes so that the time unit matches to nanoseconds?

Yes, I was sure that it's 1000, not 1000000. such a shame :)

Fix millis to nanos conversion
@atsushieno

Copy link
Copy Markdown
Owner

Awesome, thank you so much!

@atsushieno atsushieno merged commit a1d3ca2 into atsushieno:main Feb 25, 2026
0 of 2 checks passed
@e13mort e13mort deleted the jzz-fix branch February 26, 2026 17:01
@e13mort

e13mort commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

@atsushieno Hi. Do you have any plans to publish a new version with that fix?

@atsushieno

Copy link
Copy Markdown
Owner

New release would come up with the latest/updated dependencies, and currently I've been occupied by Android audio stack and have no time to verify them. You need released ktmidi packages only if you are distributing libraries. If you are just building apps, you should be able to resolve the deps by building ktmidi and installing it into mavenLocal (./gradlew publishToMavenLocal). Thanks for your understanding and patience.

@e13mort

e13mort commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Approach with mavenLocal works for me for last couple of months. So i'm not blocked. I just wanted to configure CI pipeline for my apps without extra cloning/patching steps, but at current stage it can wait.

@atsushieno

atsushieno commented May 8, 2026

Copy link
Copy Markdown
Owner
  • ktmidi
    • fix build
    • verify build on CI
    • publish to sonatype portal
  • mugene-ng
    • fix build
    • verify build on CI
    • publish to sonatype portal
  • augene-ng
    • fix build
    • verify build on CI
  • compose-audio-control
    • fix build
    • verify build on CI
    • publish to sonatype portal
  • aap-core
    • verify build (local is enough)
  • resident-midi-keyboard
    • verify build (local is enough)

@atsushieno

Copy link
Copy Markdown
Owner

ktmidi package is up on Maven Central. Thanks for your patience!

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