sugarlabs / musicblocks

Music Blocks -- A musical microworld
https://musicblocks.sugarlabs.org/
GNU Affero General Public License v3.0
558 stars 757 forks source link

Added midi support to MB #3904

Closed Mubashirshariq closed 1 month ago

Mubashirshariq commented 3 months ago

Added midi support to Music Blocks

walterbender commented 3 months ago

The first block inside the clamp of an Action Block is a Hidden Block. But the Action Blocks don't need a Hidden Block afterward. Note Blocks (all clamp blocks that can be connected to other clamp blocks) need a Hidden Block afterwards.

[ [0,["start"],988,100,[null,1,null]], [1,"settimbre",1002,141,[0,2,3,4]], [2,["voicename",{"value":"guitar"}],1162,141,[1]], [3,["nameddo",{"value":"action"}],1016,173,[1,null]], [4,"hidden",1002,236,[1,null]],

[5,["action",{"collapsed":false}],513,96,[null,6,7,null]], [6,["text",{"value":"action"}],645,105,[5]], [7,"hidden",527,137,[5,8]], [8,["newnote",{"collapsed":false}],527,137,[7,9,12,16]], [9,"divide",629,137,[8,10,11]], [10,["number",{"value":1}],715,137,[9]], [11,["number",{"value":4}],715,169,[9]], [12,"vspace",541,169,[8,13]], [13,"pitch",541,201,[12,14,15,null]], [14,["solfege",{"value":"sol"}],615,201,[13]], [15,["number",{"value":4}],615,233,[13]], [16,"hidden",527,295,[8,null]] ]

Mubashirshariq commented 3 months ago

Davy-Jones (1).zip

@walterbender i have attached the zip which contains the midi file

pikurasa commented 3 months ago

I am doing some testing today.

I imported Davy Jones, then, as I was making some corrections for you to study, something strange happened. Action blocks where connecting behind other Action blocks. Then, when I tried to move them, they disappeared completely.

I'll try to find the easiest way to reproduce this, but, for the time being, here's the last part of my logs:

Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:2129 Infinite loop checking for expandables?
blocks.js:2131 Array(519)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:2689 infinite loop finding topBlock?
blocks.js:2691 288 hidden
blocks.js:3638 Maximum loop counter exceeded in calculateDragGroup... this is bad. 295
2blocks.js:3638 Maximum loop counter exceeded in calculateDragGroup... this is bad. 296
blocks.js:2689 infinite loop finding topBlock?
blocks.js:2691 288 hidden
blocks.js:1503 Infinite loop encountered checking for expandables?
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
blocks.js:2129 Infinite loop checking for expandables?
blocks.js:2131 Array(519)
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)
11blocks.js:3638 Maximum loop counter exceeded in calculateDragGroup... this is bad. 296
blocks.js:2689 infinite loop finding topBlock?
blocks.js:2691 288 hidden
blocks.js:1503 Infinite loop encountered checking for expandables?
blocks.js:926 Did not find match for hidden (312) and newnote (313)
blocks.js:938 Array(2)
blocks.js:940 Array(4)

And full log: MIDI-test-Davy-Jones-1719264829443.log

All files I saved during testing: all-test-files.zip

pikurasa commented 3 months ago

Here's a video of a full reproduction of what I did that lead to the problem:

blocks-are-a-mess.webm

And this is the log:

Block-mess-1719265511589.log

I'll see if I can find a more succinct way to reproduce the issue.

pikurasa commented 3 months ago

I tested a different file, which seems to have exposed something strange.

Please see the behavior of the blocks in this video:

MIDI-test.webm

The file I used to create this is here:

AUD_HTX0675.zip

(I found it here: https://www.mididb.com/arijit-singh/asoch-na-sake-midi/)

pikurasa commented 3 months ago

I kind of suspect that it's not yet trimming the MIDI file in a smart way. This is because it's typically the last few action blocks that are problematic.

pikurasa commented 2 months ago

I did a test where I generated a MIDI from https://tones.wolfram.com/ and tried to import it into your latest commit (#06b7c335e).

I'm getting these errors:

activity.js:3702 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'timeSignature')
    at Activity.transcribeMidi (activity.js:3702:79)
    at midiReader.onload (activity.js:5959:26)
activity.js:5861 
t
activity.js:3702 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'timeSignature')
    at Activity.transcribeMidi (activity.js:3702:79)
    at midiReader.onload (activity.js:5862:30)
index.html:1 Access to internal resource at 'file:///home/devinulibarri/Computer-Code/musicblocks-all/walter-bender-musicblocks/android_chrome_manifest.json' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, isolated-app, chrome-extension, chrome, https, chrome-untrusted.

If it's getting a time signature of "undefined", we should probably just give it the default value of 90. (But I'm not totally certain that's what the issue is here.)

Mubashirshariq commented 2 months ago

@pikurasa this midi file doesn't contain time signatures that is why it is showing error,what default value should i keep,tempo i had already set 90 as default if it is not there

pikurasa commented 2 months ago

@pikurasa this midi file doesn't contain time signatures that is why it is showing error,what default value should i keep,tempo i had already set 90 as default if it is not there

The problem is that it's not loading the MIDI at all from within Music Blocks. Does it load for you?

Mubashirshariq commented 2 months ago

what default value should i set for timesignature array , i will add them then you test it will work .since in this case there is no timesignatures it is accessing timesignauture array which has no value it

pikurasa commented 2 months ago

Let's just do 90 when there is no timesignature specified.

pikurasa commented 2 months ago

I tested from 31ecd829a, and it works now.

walterbender commented 2 months ago

@pikurasa so ready to merge?

pikurasa commented 2 months ago

@pikurasa so ready to merge?

Yes.

pikurasa commented 2 months ago

Yes... but there is certainly still work to be done.

We seem to be getting a lot of duplicate notes, and the octave seems to be too low, in addition to the tempo being too slow.

Test from Wolfram Tones: https://youtu.be/ebcPLCI-hXw

pikurasa commented 2 months ago

Here's a test from a MIDI generated from one of my arrangements of Rudolph the Red Nosed Reindeer:

https://youtu.be/8v4W6K7S3Vk

The results are much better than from MIDI created via Wolfram Tones.

It does seem to be calculating the following two things incorrectly:

It's a bit challenging to correct, but you can see me correct it in the video.

I'm also attaching the following package of various tests:

I'll try to make a more thorough correction of the MIDI import in a bit.

pikurasa commented 2 months ago

Here's a completely corrected version: Rudolph-Corrected.zip

Notes:

@Mubashirshariq I hope this helps! If you have any questions, please let me know.

pikurasa commented 2 months ago

Here's a possible mapping give our current collection of drum sounds:

+------+--------+----------+--------+
|MIDI  |Drum    |MIDI      |Pitch   |
|number|in MB   |sound     |        |
+------+--------+----------+--------+
|35    |Bass    |Acoustic  |B1      |
|      |Drum    |Bass      |        |
|      |        |Drum      |        |
+------+--------+----------+--------+
|36    |Kick    |Electric  |C2      |
|      |Drum    |Bass      |        |
|      |        |Drum      |        |
+------+--------+----------+--------+
|37    |Cup Drum|Side      |C#2     |
|      |        |Stick     |        |
|      |        |          |        |
+------+--------+----------+--------+
|38    |Snare   |Acoustic  |D2      |
|      |Drum    |Snare     |        |
|      |        |Drum      |        |
+------+--------+----------+--------+
|40    |Snare   |Electric  |E2      |
|      |Drum    |Snare     |        |
|      |        |Drum      |        |
+------+--------+----------+--------+
|39    |Clap    |Hand      |D#2     |
|      |        |Clap      |        |
+------+--------+----------+--------+
|41    |Floor   |Low       |F2      |
|      |Tom     |Floor     |        |
|      |        |Tom       |        |
+------+--------+----------+--------+
|43    |Floor   |High      |G2      |
|      |Tom     |Floor     |        |
|      |        |Tom       |        |
+------+--------+----------+--------+
|45    |Floor   |Low Tom   |A2      |
|      |Tom     |          |        |
+------+--------+----------+--------+
|42    |Hi      |Closed    |F#2     |
|      |Hat     |Hi-hat    |        |
+------+--------+----------+--------+
|44    |Hi      |Pedal     |G#2     |
|      |Hat     |Hi-Hat    |        |
+------+--------+----------+--------+
|46    |Hi      |Open      |A#2     |
|      |Hat     |Hi-Hat    |        |
+------+--------+----------+--------+
|47    |Tom     |Low-mid   |B2      |
|      |Tom     |Tom       |        |
|      |        |          |        |
+------+--------+----------+--------+
|48    |Tom     |Hi-Mid    |C3      |
|      |Tom     |Tom       |        |
+------+--------+----------+--------+
|49    |Crash   |Crash     |C#3     |
|      |        |Cymbal    |        |
+------+--------+----------+--------+
|51    |Crash   |Ride      |D#3     |
|      |        |Cymbal    |        |
+------+--------+----------+--------+
|55    |Crash   |Splash    |G3      |
|      |        |Cymbal    |        |
+------+--------+----------+--------+
|57    |Crash   |Crash     |A3      |
|      |        |Cymbal    |        |
+------+--------+----------+--------+
|59    |Crash   |Ride      |B3      |
|      |        |Cymbal    |        |
+------+--------+----------+--------+
|50    |Tom     |High Tom  |D3      |
|      |Tom     |          |        |
+------+--------+----------+--------+
|52    |Finger  |Chinese   |E3      |
|      |Cymbals |Cymbal    |        |
+------+--------+----------+--------+
|53    |Ride    |Ride      |F3      |
|      |Bell    |Bell      |        |
+------+--------+----------+--------+
|60    |Darbuka |Hi Bongo  |C4      |
|      |Drum    |          |        |
+------+--------+----------+--------+
|62    |Darbuka |Mute Hi   |D4      |
|      |Drum    |Conga     |        |
+------+--------+----------+--------+
|63    |Darbuka |Open Hi   |Eb4     |
|      |Drum    |Conga     |        |
+------+--------+----------+--------+
|65    |Darbuka |High      |F4      |
|      |Drum    |Timbale   |        |
+------+--------+----------+--------+
|67    |Raindrop|High      |G4      |
|      |        |Agogo     |        |
+------+--------+----------+--------+
|69    |Raindrop|Cabasa    |A4      |
|      |        |          |        |
+------+--------+----------+--------+
|56    |Cow     |Cowbell   |G#3     |
|      |Bell    |          |        |
+------+--------+----------+--------+
|64    |Japanese|Low       |E4      |
|      |Drum    |Conga     |        |
+------+--------+----------+--------+
|66    |Japanese|Low       |F#4     |
|      |Drum    |Timbale   |        |
+------+--------+----------+--------+
|68    |Japanese|Low Agogo |G#4     |
|      |Drum    |          |        |
+------+--------+----------+--------+
|73    |Cup Drum|Short     |D#5     |
|      |        |guiro     |        |
+------+--------+----------+--------+
|71    |Gong    |Short     |B4      |
|      |        |Whistle   |        |
+------+--------+----------+--------+
|72    |Gong    |Long      |C4      |
|      |        |Whistle   |        |
+------+--------+----------+--------+
|76    |Slap    |Hi Wood   |E5      |
|      |        |Block     |        |
+------+--------+----------+--------+
|77    |Slap    |Low Wood  |F5      |
|      |        |Block     |        |
+------+--------+----------+--------+
|70    |Clang   |Maracas   |A#4     |
|      |        |          |        |
+------+--------+----------+--------+
|54    |Chime   |Tambourine|F#3     |
|      |        |          |        |
+------+--------+----------+--------+
|58    |Chime   |Vibraslap |A#3     |
|      |        |          |        |
+------+--------+----------+--------+
|78    |Chime   |Mute Cuica|F#5     |
|      |        |          |        |
+------+--------+----------+--------+
|79    |Chime   |Open Cuica|G5      |
|      |        |          |        |
+------+--------+----------+--------+
|80    |Triangle|Closed    |G#5     |
|      |Bell    |Triangle  |        |
+------+--------+----------+--------+
|81    |Triangle|Open      |A5      |
|      |Bell    |Triangle  |        |
+------+--------+----------+--------+

A MB file with the pitch mappings: Conversion-MIDI-reviewed.zip

Note: Although this has had a review, there may still be some errors. Even so, it's not a perfect science, so feel free to make changes.

pikurasa commented 1 month ago

I did some testing, and it may be that something isn't quite working right with "map pitch to drum".

For example, F#2, G#2, and A#2 should all be converted to "hi hat", but they are not doing that right now.

(I'm wondering if it's skipping sharps, but I haven't tested enough to be certain of that.)

@walterbender I think we can merge this and work on that issue separately.

@Mubashirshariq if you have any bandwidth, you could look into "map pitch to drum" and see why some of the pitches, despite being spelled out in our big action block, are not being mapped to drums. This work can be done in a separate branch, if you do it.

Mubashirshariq commented 1 month ago

Yeah sure I will open a separate PR for tha

walterbender commented 1 month ago

@Mubashirshariq could you please rebase from master to resolve the merge conflicts. Thx.

Mubashirshariq commented 1 month ago

@walterbender i have solved the merge confilct