Skip to content
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

ABC conversion to MB #3837

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

ABC conversion to MB #3837

wants to merge 21 commits into from

Conversation

gitjeet
Copy link

@gitjeet gitjeet commented Mar 24, 2024

-Converts .abc to MB

@walterbender
Copy link
Member

Question: Why do you convert to Solfege? If you do that conversion, you need to account for the key/mode.

@gitjeet
Copy link
Author

gitjeet commented Mar 27, 2024

@walterbender fixed this issue

@walterbender
Copy link
Member

Devin spotted an issue with Happy Birthday. Are there ways to accommodate key/mode you are not considering?

@walterbender
Copy link
Member

Any progress on this?

@gitjeet
Copy link
Author

gitjeet commented Apr 8, 2024

@walterbender yes i am working on next step ie abc parser consider the key which the music is in

@walterbender
Copy link
Member

Some sample test data are found here: https://abcnotation.com/examples

@gitjeet gitjeet changed the title Added Simple ABC conversion to MB ABC conversion to MB Apr 10, 2024
@gitjeet
Copy link
Author

gitjeet commented Apr 10, 2024

@walterbender @pikurasa
The above feature ABC conversion to MB could be breakdown to following milestones

  • complete implmentation of ABC parser for Music which takes consideration of all parameter such key, pitch,notes,chords

  • take consideration of repetion and break the MB to modular

  • implement the state design pattern

could you let me know if anything to add above

@gitjeet
Copy link
Author

gitjeet commented Apr 10, 2024

Some sample test data are found here: https://abcnotation.com/examples

i have tested it with simple once i will test with long and complex composition where it could break the conversion

@walterbender
Copy link
Member

The space between the accidental and the letter needs to be removed.

js/activity.js Outdated
});

if (accidental) {
return note + (accidental.acc === "sharp" ? " ♯" : (accidental.acc === "flat" ? " ♭" : ""));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"sharp" ? "♯"
"flat" ? "♭"

@walterbender
Copy link
Member

@pikurasa can you test again? Key signature seems to be working now.

@pikurasa
Copy link
Collaborator

@pikurasa can you test again? Key signature seems to be working now.

Will do. Fams been sick (again!), so it's been slow going... but I have time at the moment.

@pikurasa
Copy link
Collaborator

@gitjeet Please remind me how to test this.

I've tried:

  1. Opening a .abc file
  2. Copying abc code from abcnotation.com/examples and pasting it into MB
  3. Dropping copied code into MB

None of these seem to work...

@gitjeet
Copy link
Author

gitjeet commented Apr 15, 2024

VID_20240415_232605.mp4

Hi @pikurasa we just need to drag and drop the abc file to MB, could you also check you are on the right branch ?

@pikurasa
Copy link
Collaborator

I am on commit 833d866.

Thanks for the video -- that works!

I recommend that we also make a few of those other options available, too, such as opening a .abc or using "ctl-v" to paste the code in and import it that way.

Meanwhile, I'm reviewing (thanks again!)

@pikurasa
Copy link
Collaborator

(I found plenty of feedback, and I'm working on a helpful way to show the feedback. For example, it doesn't seem to account for accidentals, when those accidentals are in the key. I'll upload some examples soon.)

@pikurasa
Copy link
Collaborator

So, this is what I tested:

I tested Amazing Grace, found here: https://abcnotation.com/tunePage?a=thesession.org/tunes/2749.no-ext/0003

It opened as the following:

Screenshot from 2024-04-15 14-26-38

There's a lot of good that's in here, but the following needs to be fixed:

(Roughly in order of importance)

  1. Separate voices are delineated as "V:1" and "V:2" (for "Voice 1" and "Voice 2"), so they should be put into separate Start Blocks. Right now, it is putting them all into the same stack, which is a problem since it puts the music out of order.
  2. The pitches for in the key are ignored. This piece is in D Major. I'm not super familiar with ABC notation, but I did review its documentation just now, and it seems that pitches are automatically adjusted to their accidentals based on the key given. That's different than how we do it -- and that's regardless of solfege or alphabet**. Therefore, for ABC --> MB conversion, we need to make the accidental explicit.
  3. Meter -- It should be easy enough to add a meter block based on the code at the top. In this case, M: 3/4 should be interpreted as Meter; number of beats = 3; note value = 1/4.
  4. Lines -- If we want, we could break the music up into separate action blocks, based on whether the .abc file has the text on a new line
  5. Chords -- The pitches that are contained within quotation marks represent chords. If we want, we could generate a separate Start Block with those chords.

Here are some files to help:

A simple fix of accidentals given in key

Screenshot from 2024-04-15 15-18-27
amazing-grace-fix-f-sharp.zip

A fix with each line contained within a separate action block

Screenshot from 2024-04-15 16-05-46

Amazing-Graze-Lines-Broken.zip

A fix with each line contained within a separate action block, plus another start block with chords

Screenshot from 2024-04-15 16-04-51

Amazing-Grace-ABC-with-chords.zip

Note: I'm not really sure how to do a sus chord in MB at the moment, so I just put "custom" in as a placeholder. I'm sure some of these other chords would need some explanation. Also, we seem to be mixing fixed systems (e.g. major, minor) with moveable (e.g. triad, seventh, etc) in how we do chords, so that really needs some more digging into on our part.

2nd Note: Since the first and third lines have the same progression, I just used the same action block.

**Re: solfege (Do, Re, Mi) and alphabet (C, D, E) -- again, solfege can be either moveable or fixed; alphabet is only fixed. In this light, I don't quite understand Walter's comment above. You could do either solfege or fixed. Both fixed solfege and alphabet need to be handled the same way. Handling solfege (or scale degree, which works the same way) would need to be handled differently. It may be nice to give the user a choice whether they want to convert to (fixed) solfege or alphabet (presumably, we would not be converting to a moveable system until we get fixed working). At any rate, what needs to be done now in terms of conversion of ABC to MB-alphabet is taking the original key into consideration, so, for example, if there are F sharps in the key signature, all the Fs need to be sharped.

Hopefully this feedback helps. Please let me know if you have any questions.

@walterbender
Copy link
Member

I made my original comment about solfege vs letter when the code was not taking into account the key signature.

Re suspended chords, we could define them. We don't have them in the predefined set at the moment.

@pikurasa
Copy link
Collaborator

Re suspended chords, we could define them. We don't have them in the predefined set at the moment.

Ok, but how does one create a custom chord at the moment? I know how to define a custom scale, but not chord...

@gitjeet
Copy link
Author

gitjeet commented Apr 16, 2024

Thanks @pikurasa for the feedback. I will start working on fixing and improving it.

@gitjeet
Copy link
Author

gitjeet commented Apr 24, 2024

@walterbender @pikurasa The ABC conversion to MB is almost completed everything is implemented except the chord feature and some name issue is pending,
image
for lowecase letter notes i was thinking to change into standard upper case letter as in MB because in abc it means to lower the octave but i think in MB we have uppercase letter format

@pikurasa
Copy link
Collaborator

The ABC conversion to MB is almost completed everything is implemented except the chord feature and some name issue is pending,

I don't think you need to do chords for this PR.

for lowecase letter notes i was thinking to change into standard upper case letter as in MB because in abc it means to lower the octave but i think in MB we have uppercase letter format

In MB, we use number for octave. I'd need to do some more research to see how ABC maps to our octave naming system.

@gitjeet
Copy link
Author

gitjeet commented Apr 24, 2024

image
@pikurasa @walterbender Could you please review this pull request again? I've implemented all the changes as requested.

@pikurasa
Copy link
Collaborator

I tested with https://abcnotation.com/tunePage?a=www.atrilcoral.com/Partituras_ABC/l.zip/l/0854 and:

  1. It doesn't seem to be breaking up different lines into separate action blocks
  2. It doesn't seem to be catching the C#'s (although it is concerting the F#'s)
  3. It's not yet converting tuplets properly (I'll investigate this a bit further to see if I can see what the conversion should be)

This all said, you are doing great work here.

@pikurasa
Copy link
Collaborator

Re: tuplets, unsure when I'll be able to do a more comprehensive review, but for starters:

The group of notes (3Bcd in this piece should be interpreted as 1/12 note values in Music Blocks.

An aside: I greatly prefer the way we handle note lengths to anything else I've seen so far. It's so much more transparent.

@gitjeet
Copy link
Author

gitjeet commented Apr 26, 2024

Thanks, @pikurasa, for the feedback and kind words. I will make the necessary changes

@pikurasa
Copy link
Collaborator

@gitjeet where is progress on this? I'd love to be able to merge this as I expect it to be very useful.

@gitjeet
Copy link
Author

gitjeet commented May 27, 2024

@pikurasa i am working on this will try to wrap this by this week

@gitjeet
Copy link
Author

gitjeet commented May 27, 2024

I tested with https://abcnotation.com/tunePage?a=www.atrilcoral.com/Partituras_ABC/l.zip/l/0854 and:

1. It doesn't seem to be breaking up different lines into separate action blocks

2. It doesn't seem to be catching the C#'s (although it is concerting the F#'s)

3. It's not yet converting tuplets properly (I'll investigate this a bit further to see if I can see what the conversion should be)

This all said, you are doing great work here.

dBAF DEFD|G3A BEE2|dBAF DEFA|(3Bcd AG FDD2:||\ d3e f3d|efed cAA2|(3Bcd ef g2ag|fgeg fddc:||\ (3Bcd Ad (3Bcd Ad|(3Bcd ef gfed|(3Bcd Ad (3Bcd ag|fgeg fddc:||

  1. the abc notion for this contains \ i checked this this means treat the next line as a continuation of the current line, rather than starting a new musical phrase because of this adding it adds all in single line, i checked and removed \ works fine

  2. i checked the c# are there could you let me know if missing anything
    image

  3. could you let me know t triplet also depends on time signature so if 3/4 time signature what each note would get the same beats 1/12, ?

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.

None yet

4 participants