Skip to content

Conversation

@therealharshit
Copy link
Member

This feature was suggested by @pikurasa in #4345

Vedio:

drop-2.mp4

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@therealharshit therealharshit marked this pull request as draft February 22, 2025 20:17
@therealharshit
Copy link
Member Author

@walterbender everything seems to be working fine on the UI side but the audio sample is not playing,

here are the logs:

0 261 0.5 electronic synth null null false undefined
synthutils.js:1756 0 220 1 customsample_piano.wav_original null null false undefined
synthutils.js:1811 customsample_piano.wav_original
synthutils.js:1558 Error: buffer is either not set or not loaded
at Bn (VM14 Tone.js:2:113676)
at Lo.start (VM14 Tone.js:2:194089)
at VM14 Tone.js:2:278174
at Array.forEach ()
at Yr.triggerAttack (VM14 Tone.js:2:277894)
at Yr.triggerAttackRelease (VM14 Tone.js:2:279077)
at Synth._performNotes (synthutils.js:1555:23)
at Synth.trigger (synthutils.js:1840:22)
at SampleWidget._playSample (sampler.js:628:38)
at sampler.js:647:22

what could be the posible fix for this?

@Commanderk3
Copy link
Member

Commanderk3 commented Feb 23, 2025

@therealharshit I think the way you are passing values to "this.makeSamplerWidget" is not effective.
We need to pass the .wav file itself but that is handle by functions like "handleFiles" present in sampler widget code.

@Commanderk3
Copy link
Member

I am interested in this, if you agree we can work together on this one.

@therealharshit
Copy link
Member Author

@therealharshit I think the way you are passing values to "this.makeSamplerWidget" is not effective. We need to pass the .wav file itself but that is handle by functions like "handleFiles" present in sampler widget code.

I worked on handelFiles and I don't think we need that here when we can directly pass the data to sampleWidget.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@therealharshit therealharshit marked this pull request as ready for review February 23, 2025 18:52
@therealharshit
Copy link
Member Author

@walterbender I have fixed the issue which I was facing to play the sample files, now everything is working as suggested by @pikurasa .

Vedio:

Sampler-drop.mp4

@Commanderk3 actually the issue was with the FileReader it was initialized to readAsText but for audio/wav type it's required to readAsDataURL thats why it was unable to read the data of wav file in correct format, I made the changes and specified it to readAsDataURL only for audio/wav file type.

@Commanderk3
Copy link
Member

@therealharshit Yeah, nice work. I kind of reached the bottom of this problem but didn't know how readAsText and readAsDataURL works.

@Commanderk3
Copy link
Member

Commanderk3 commented Feb 23, 2025

Still i found one more issue. The existing blocks disappear when you drop the file. Did you notice?

@therealharshit
Copy link
Member Author

Still i found one more issue. The existing blocks disappear. Did you notice?

Yeah it's the default behavior of MB to clean canvas when a file is dropped, and since I used the existing workflow to implement this feature it's working as it should.
But i think for this we don't require to clean the canvas, maybe lets see what @walterbender @pikurasa suggests.

@walterbender
Copy link
Member

We should not remove existing blocks when dropping a sound file onto the canvas. (Not sure we should even do it for project files, but that is a discussion for another time.)

@Commanderk3
Copy link
Member

Commanderk3 commented Feb 23, 2025

It does not happen when you drop midi files though.

@therealharshit
Copy link
Member Author

We should not remove existing blocks when dropping a sound file onto the canvas. (Not sure we should even do it for project files, but that is a discussion for another time.)

@walterbender there is a event trashsignal in the try block, I think maybe that is responsible for removing existing blocks.
Can you please tell me what is trashsignal , is it a kind of custom event?

@walterbender
Copy link
Member

trashsignal is invoked inside sendAllToTrash(). So the question is, why is sendAllToTrash() being called?

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@therealharshit
Copy link
Member Author

We should not remove existing blocks when dropping a sound file onto the canvas. (Not sure we should even do it for project files, but that is a discussion for another time.)

@walterbender for now I fixed it for sound sample.

vedio:

sampler-drop.2.mp4

Please review and let me know.

@walterbender walterbender merged commit 728ecf4 into sugarlabs:master Feb 24, 2025
5 checks passed
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.

3 participants