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

[musicxml2hum] Wrong note duration before spine merges #89

Closed
WolfgangDrescher opened this issue Dec 8, 2023 · 2 comments
Closed

[musicxml2hum] Wrong note duration before spine merges #89

WolfgangDrescher opened this issue Dec 8, 2023 · 2 comments

Comments

@WolfgangDrescher
Copy link
Contributor

Because #88 is getting a bit long I'm opening a new issue for another bug I found in musicxml2hum as it is not directly related to the mentioned PR.

Here is an excerpt from the import of another Schiørring chorale (87):

VHV

**kern	**kern
*part5	*part5
*staff6	*staff5
*mclefF4	*mclefG2
*k[b-e-a-]	*k[b-e-a-]
*	*
*^	*^
*	*	*	*^
=1	=1	=1	=1	=1
2G	2C	2cc	2g	2e-\
=2	=2	=2	=2	=2
2G	2E-	2g	2.c	2e-/
*	*	*	*v	*v
4A-	2D	2f	.
4G	.	.	4Bn
*	*	*v	*v
*	*v	*v
*-	*-

The syntax is invalid, because on line 12 2.c and 2e-/ do not have the same duration.

I see two solutions for this:

VHV

**kern	**kern
*part5	*part5
*staff6	*staff5
*mclefF4	*mclefG2
*k[b-e-a-]	*k[b-e-a-]
*	*
*^	*^
*	*	*	*^
=1	=1	=1	=1	=1
2G	2C	2cc	2g	2e-\
=2	=2	=2	=2	=2
2G	2E-	2g	2.c	2e-/
4A-	2D	2f	.	4ryy
4G	.	.	4Bn	4ryy
*	*	*	*v	*v
*	*	*v	*v
*v	*v	*
*-	*-

or when the voices of a spine split would be arranged in a different order:

VHV

**kern	**kern
*part5	*part5
*staff6	*staff5
*mclefF4	*mclefG2
*k[b-e-a-]	*k[b-e-a-]
*	*
*^	*^
*	*	*^	*
=1	=1	=1	=1	=1
2G	2C	2cc	2g	2e-\
=2	=2	=2	=2	=2
2G	2E-	2g	2e-	2.c\
*	*	*v	*v	*
4A-	2D	2f	.
4G	.	.	4Bn
*	*	*v	*v
*v	*v	*
*-	*-

What do you think would be the best solution for this? Both variants do not look as if they would be easy to implement...

@craigsapp
Copy link
Owner

I think solution no. 1 is best, although with a variant:

**kern	**kern
*part5	*part5
*staff6	*staff5
*mclefF4	*mclefG2
*k[b-e-a-]	*k[b-e-a-]
*	*
*^	*^
*	*	*	*^
=1	=1	=1	=1	=1
2G	2C	2cc	2g	2e-\
=2	=2	=2	=2	=2
2G	2E-	2g	2.c	2e-/
4A-	2D	2f	.	4ryy
*	*	*	*v	*v
4G	.	.	4Bn
*	*	*v	*v
*v	*v	*
*-	*-

Your original solution for no. 1 is also acceptable: I typically will fill in the full measure of partial voices with invisible rests to make it easier to read the data.

I would perhaps try to minimize the number of invisible rests (not too important):

**kern	**kern
*part5	*part5
*staff6	*staff5
*mclefF4	*mclefG2
*k[b-e-a-]	*k[b-e-a-]
*	*
*^	*^
*	*	*	*^
=1	=1	=1	=1	=1
2G	2C	2cc	2g	2e-\
=2	=2	=2	=2	=2
2G	2E-	2g	2.c	2e-/
4A-	2D	2f	.	2ryy
4G	.	.	4Bn	.
*	*	*	*v	*v
*	*	*v	*v
*v	*v	*
*-	*-

And it would be OK to merge all three voices at once at the end of the measure:

**kern	**kern
*part5	*part5
*staff6	*staff5
*mclefF4	*mclefG2
*k[b-e-a-]	*k[b-e-a-]
*	*
*^	*^
*	*	*	*^
=1	=1	=1	=1	=1
2G	2C	2cc	2g	2e-\
=2	=2	=2	=2	=2
2G	2E-	2g	2.c	2e-/
4A-	2D	2f	.	2ryy
4G	.	.	4Bn	.
*	*	*v	*v	*v
*v	*v	*
=	=
*-	*-

@WolfgangDrescher
Copy link
Contributor Author

I think solution no. 1 is best, although with a variant:

Yes, this was a mistake I did when merging the spines manually. I thought I had fixed it, but I seem to have forgotten the first example.

Your original solution for no. 1 is also acceptable:

You are right, if I render this example with Verovio directly it works perfectly fine. If this is valid Kern, I think I will not change musicxml2hum. I just copy&pasted the Kern to VHV and it did not render, because an error "Note durations do not match for designated join spines" was raised and VHV refused to render the score. Should I try to change this in VHV to raise a warning instead?

In general it would be nice to have a humvalidate tool in C++. I will add this to my to-do list. Is https://github.com/humdrum-tools/verovio-humdrum-viewer/blob/gh-pages/scripts/local/humdrumValidator.js a good starting point for this? humvalidate could also check things like the order of tokens in data records: 2.c vs 2c..

@WolfgangDrescher WolfgangDrescher closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2023
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

No branches or pull requests

2 participants