-
Notifications
You must be signed in to change notification settings - Fork 35
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
Record audio button #113
Record audio button #113
Conversation
TODO:
|
You can base64 encode the MIDI and stick it in a query param. It's fine if it is too long, since it'll only be used client side (servers never need to process it). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played with the functionality and it worked well! Thank you for the contributions so far. Excited to see what you do with the "save as" dialog 😄
If you'd like, feel free to install and use the Dialog component from Radix-UI
src/features/audioRecording/index.ts
Outdated
if (isAble) { | ||
const recordingDestinationNode = audioContext.createMediaStreamDestination(); | ||
setRecordingDestinationNode(recordingDestinationNode); | ||
mediaRecorder = new MediaRecorder(recordingDestinationNode.stream, {mimeType: 'audio/webm'}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we default to mp3
instead of webm
? I think most people using the feature would be surprised to have webm as the default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried but the file somehow comes corrupted. I guess we have to convert it.
I've made the midi recording (it only records midi message events, not pressing notes with mouse) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solved now by updating useSong
to understand base64 as a source type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EloxZ, thank you for the updates! I didn't know about @tonejs/midi
, but that seems like it'll be very useful for sightread.
In fact, I think it'd probably be better to replace the implementation of parsers/parseMidi
to use @tonejs/midi
instead of jasmid since it so much higher level.
Separately, is it possible you forgot to push your latest updates? I don't see any code right now that adds to the recorededMidiEvents
array. Testing this resulting in an empty list
Let me know if there's anything you need help with to get this to the finish line. Some things I think it needs before it's ready:
- Remove all the
console.log
statements and unused code / some mild code cleanup - It needs to function correctly
- Ideally a prompt to ask if you'd like to copy as a link or download as a webm/mp3, and where to save it to.
I can work on the parseMidi migration out of band
@EloxZ, let me know if you'd like to continue working on this. Otherwise I'll pick it up next week |
I can continue. Did you do something? |
Not yet! I've been busy with other projects |
Added button for recording audio in freeplay, it downloads a .webm file for broad browser support
Fix format
ce71415
to
55e9fff
Compare
* Small bugfixes to the Player. Restarts song when at the end and hitting play. * Removes support for downloading mp3-style audio. MIDI is enough for this PR, future one can use lamemp3 etc to convert the midi to mp3.
fad2caf
to
cd72997
Compare
7aab9f1
to
0787d27
Compare
@EloxZ I've made some updates, and plan to merge it now. I've also removed the Thank you again for kicking this off 😄 |
#110