Fix #15899: MusicXML figured bass support

[jsyleung]

Updated figured bass export and import according to the MusicXML 3.0 specification. Specifically, the following features were changed/added:

• Figures held over multiple notes are no longer treated as prolonged durations, but exclusively as extension lines (is backward compatible).
  • Extend tags now specify type.
• Changing figures under a single note can now be exported and imported, utilizing the duration tag.
• The “cross” and “backslash” modifiers can now be exported and imported as prefix or suffix tags.

[chenlung]

Hi @algadoreo

I understand if you only want to focus on figured bass, but since we are on the subject of extenders in MusicXML, I'm curious to know your opinion on this?: http://musescore.org/en/node/46486

Thanks
Scott

[jsyleung]

@chenlung
If there still aren’t any implementations to export (import) melismas to (from) MusicXML, I would recommend updating it using extend tags as well. It shouldn’t be a difficult fix, as the idea is the same in both lyrics and figured bass; the difference is only in how they’re represented within the MuseScore code.

[MarcSabatella]

FYI, according to the Travis build log, the automated test failed - tst_mxml_io.

[jsyleung]

@MarcSabatella
It is failing because the output file does not exactly match the input (which is what that test does). I’m certain the culprit is not the figured bass, but with certain layout parameters – specifically, the <credit-words … default-y="…" … > and <top-system-distance> tags change values every time I import/export the same file repeatedly. (I manually tested this with a fresh clone of the main branch (4eda1f6) and my working copy, with the same result) That said, the ordering of some tags has been changed since I last rebased (a week ago), so those are other places where the the test output wouldn’t match the input – that file should be updated.

On the other hand, I also forgot to add the new test files to the list in tst_mxml_io.cpp, but this just means those weren’t tested. But in all likelihood, the result would be the same: the layout would be messed up.

I await your further instructions. Thanks!

[MarcSabatella]

I'm not really sure how these tests work myself, but did you try simply editing the XML to remove those tags?

[jsyleung]

As far as I could tell, all this test does is import the test file, (re-)export it, then check if they are the same. Leon (@lvinken) would know about this better than I do – by far.

Is there a way to run this particular test automatically, say, via command line?

[AntonioBL]

The tests you changed are not the ones which are actually automatically tested by Travis.
You changed some old tests in folder test/musicxml/ while the automatic tests are in mtest/musicxml/io.
To run the tests you should compile MuseScore in debug mode ("debug" option during make, e.g. "make debug"), then navigate to the build.debug/mtest folder and launch "make" (this will compile all the tests; you can also navigate into one single test folder inside build.debug/mtest to check only one specific test). After compilation, you can launch the tests. Note that they require "diff" command (I still have lots of problem in runngin them under Windows, but under Linux everything is straight forward).
If I managed to correctly apply your changes, here is the interesting part of the output log of the tests:
https://gist.github.com/AntonioBL/9cf7a4807f9970d09885#file-gistfile1-txt
Are there some empty "figure" tags?
Edit (1):
I think the empty tags are probably created because the input file does not contain all the "continue" or "stop" informations (?)

Edit (2):
If you want to create new tests, you should remember to save the file when launching MuseScore with the "-t" (=enable test mode) option. See http://musescore.org/en/node/38301

[jsyleung]

Ah! So I’ve been putting the test files in the wrong directory – that would explain a lot! I also wasn’t saving while in test mode, which is why I’m getting all the extraneous formatting tags. Running tst_mxml_io now (within Xcode) returns this:

> QDEBUG : TestMxmlIO::figuredBass1() importMusicXml(0x10883fa00, /Users/jleung/Documents/code/git/MuseScore/mtest/musicxml/io/testFiguredBass1.xml)
> QDEBUG : TestMxmlIO::figuredBass1() Validation time elapsed: 84 ms
> QDEBUG : TestMxmlIO::figuredBass1() importMusicXml() file '/Users/jleung/Documents/code/git/MuseScore/mtest/musicxml/io/testFiguredBass1.xml' is a valid MusicXML file
> QDEBUG : TestMxmlIO::figuredBass1() score-part:part-list:score-partwise:: Node not implemented: <midi-device>, type 1
> QDEBUG : TestMxmlIO::figuredBass1() Parsing time elapsed: 2 ms
> QDEBUG : TestMxmlIO::figuredBass1() importMusicXml() return 0
> PASS   : TestMxmlIO::figuredBass1()
> QDEBUG : TestMxmlIO::figuredBass2() importMusicXml(0x10883fa00, /Users/jleung/Documents/code/git/MuseScore/mtest/musicxml/io/testFiguredBass2.xml)
> QDEBUG : TestMxmlIO::figuredBass2() Validation time elapsed: 128 ms
> QDEBUG : TestMxmlIO::figuredBass2() importMusicXml() file '/Users/jleung/Documents/code/git/MuseScore/mtest/musicxml/io/testFiguredBass2.xml' is a valid MusicXML file
> QDEBUG : TestMxmlIO::figuredBass2() score-part:part-list:score-partwise:: Node not implemented: <midi-device>, type 1
> QDEBUG : TestMxmlIO::figuredBass2() score-part:part-list:score-partwise:: Node not implemented: <midi-device>, type 1
> QDEBUG : TestMxmlIO::figuredBass2() Parsing time elapsed: 3 ms
> QDEBUG : TestMxmlIO::figuredBass2() importMusicXml() return 0
> PASS   : TestMxmlIO::figuredBass2()
> QDEBUG : TestMxmlIO::figuredBass3() importMusicXml(0x10883fa00, /Users/jleung/Documents/code/git/MuseScore/mtest/musicxml/io/testFiguredBass3.xml)
> QDEBUG : TestMxmlIO::figuredBass3() Validation time elapsed: 155 ms
> QDEBUG : TestMxmlIO::figuredBass3() importMusicXml() file '/Users/jleung/Documents/code/git/MuseScore/mtest/musicxml/io/testFiguredBass3.xml' is a valid MusicXML file
> QDEBUG : TestMxmlIO::figuredBass3() score-part:part-list:score-partwise:: Node not implemented: <midi-device>, type 1
> QDEBUG : TestMxmlIO::figuredBass3() Parsing time elapsed: 7 ms
> QDEBUG : TestMxmlIO::figuredBass3() importMusicXml() return 0
> PASS   : TestMxmlIO::figuredBass3()

I’ve updated my development branch with the changes, so we’ll see what Travis says. Thanks everyone!

EDIT: It looks like the current Travis build is failing in general, but at least tst_mxml_io passed.

[lasconic]

Yes, the test on the master branch was failing. If you rebase, it should be ok.

[lvinken]

@AntonioBL is right, the test directory is old (and probably even obsolete, at least the MusicXML tests have not been updated or used for years). The mtest directory contains the current tests, as executed by Travis. The reference files must be created in debug ("-d") mode, which a.o. suppresses layout info.

And yes, all tests in mtest/musicxml/io simply read and write a file and verify the resulting output file is as expected. Even though simple in nature, this has already caught countless regression that broke the MusicXML import export without causing compile errors.

[lvinken]

Had a quick look at the code (but did not compile and test it). Could not find any major issues.

I don't understand the purpose of mscore/importxml.cpp line 5842 "fb->setTicks(fb->ticks());"
The change to test/musicxml/testfiles_MusicXML_regular is superfluous but harmless

Did anyone verify if testFiguredBass[1-3].xml import correctly into one of our competitors products ? I only have Finale NotePad which ignores figured bass.

[jsyleung]

Right – the automatic test (tst_mxml_io) passes after I removed the layout info and copied the test files to mtest/musicxml/io/, its results posted previously. Thank you for pointing me there – I would not have found it otherwise. I kept and updated the files in the (deprecated) test/ directory just in case someone decides to activate it again.

Addressing @lvinken's questions/concerns:

• If I recall properly, that line (5842) was a remnant of my many attempts in importing changing figures. After testing, I agree that line serves no purpose and have removed it.
• The change to test/musicxml/testfiles_MusicXML_regular was, again, just to be consistent, even if that directory has been deprecated.
• To the best of my knowledge, only MuseScore interprets <figured-bass> tags. In fact, neither Finale nor Sibelius has a dedicated figured bass object – the former simply treats them as lyrics, while the latter uses Roman numerals.

I will push the updates once the main Travis build is working again – as of writing, its status is “error.”

[lasconic]

Travis build should be ok now.

[jsyleung]

@lasconic:

Travis build should be ok now.

True as that may be, I seem to have terrible luck with it. Could someone nudge a rebuild please?

[lasconic]

OK tests are passing now.

There is one file in the Lilypond test suite for MusicXML https://github.com/tomkrush/TAMusic/blob/master/musicXML.bundle/74a-FiguredBass.xml but it doesn't contain figured bass. It would be great to know if we can still open it after these changes.

Two more things,
1/ I don't think it's necessary to add or modify files in the test directory. You could probably revert this changes.
2/ Could you squash all the commits together? http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html so we have a cleaner history in MuseScore main repository. Thanks !

[jsyleung]

Done: files in the \test directory have been reverted and all the commits are squashed into one.

The Lilypond test file runs just fine because it does not involve anything I implemented. The behaviour is exactly the same before/after my changes, down to the same error thrown by the purposefully ill-formed <figured-bass> tag.

[lasconic]

@lvinken this PR is good to go so I'll merge it. Not sure how far you are with your migration to the new parser but unfortunately that might collide a bit.
Other things that could collide once merged: #1536

[lvinken]

Won't hurt too much, but thanks for reminding me. Pull parser now contains
about 2700 lines of code. DOM parser is more than 7000. Still a significant
about of work to do, which unfortunately is not simply a straightforward
replace of a few function calls.

On Tue, Feb 17, 2015 at 3:29 PM, Nicolas Froment notifications@github.com
wrote:

@lvinken https://github.com/lvinken this PR is good to go so I'll merge
it. Not sure how far you are with your migration to the new parser but
unfortunately that might collide a bit.
Other things that could collide once merged: #1536
#1536

—
Reply to this email directly or view it on GitHub
#1733 (comment).