search

grame-cncm / libmusicxml

A C/C++ library to support the MusicXML format.
Mozilla Public License 2.0
152 stars 33 forks source link

xml2guido crash #40

Closed dfober closed 4 years ago

dfober commented 4 years ago

conversion crashes with the following file: PartGroups_LOOP.xml.zip

arshiacont commented 4 years ago

The MusicXML is weird! There's a Wedge start without a stop and I assumed there is always one (which there should otherwise the XML is not valid). I can certainly fix that by for example ignoring them (I do this anyway for Slurs who start on one voice and finish on another in XML as they can not be handled in Guido). But I'm curious if you can send me the unedited version if it also crashes.

dfober commented 4 years ago

I don't know where this file comes from. If it's not too complicated, it would be good to avoid crashes as much as possible (there's nothing like a very robust code).

jacques-menu commented 4 years ago

Hello Arshia,

PartGroups_LOOP.xml looks like the name of file produced by ‘xml2ly -loop PartGroups.xml’, which I’ve been testing some time ago. Did you find it in the git repository?

JM

Le 27 mai 2020 à 07:44, Dominique Fober notifications@github.com a écrit :

I don't know where this file comes from. If it's not too complicated, it would be good to avoid crashes as much as possible (there's nothing like a very robust code).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

arshiacont commented 4 years ago

@jacques-menu It comes from Dominique and I assume it comes from the repository.

I will fix this and other similar issues and send a PR (I have lots of improvements elsewhere for xml2guido and it's time anyway).

Do we have a specific Policy in LibMusicXML for throwing Exceptions for example in case of error?

dfober commented 4 years ago

No, there isn't. But we could define one.

arshiacont commented 4 years ago

ok.. this is now fixed on my local copy and will be sent as a PR (for xml2guido 3.1 including other features and fixes).

Ideally a convertor should never crash and should send errors, either as a library or application. I am not that much familiar with C++ Exceptions and their behaviour in libraries. But this is what I'd do in Swift and similar, starting to define an Enum like structure with common Error Types.

arshiacont commented 4 years ago

@dfober This is now PullRequest #41 including this fix and lots of others. It is in production on our services and tested (on OSX/iOS). Let me know if there's any problem.

jacques-menu commented 4 years ago

Hello Arshia,

FYI, here is what I’ve just done for the lilypond branch for what it’s worth, avoiding an enum:

https://github.com/grame-cncm/libmusicxml/blob/lilypond/src/utilities/exceptions.h https://github.com/grame-cncm/libmusicxml/blob/lilypond/src/utilities/exceptions.h

https://github.com/grame-cncm/libmusicxml/blob/lilypond/src/utilities/messagesHandling.cpp https://github.com/grame-cncm/libmusicxml/blob/lilypond/src/utilities/messagesHandling.cpp

https://github.com/grame-cncm/libmusicxml/blob/lilypond/samples/xml2ly.cpp https://github.com/grame-cncm/libmusicxml/blob/lilypond/samples/xml2ly.cpp

https://github.com/grame-cncm/libmusicxml/blob/lilypond/src/lilypond/musicxml2lilypond.cpp https://github.com/grame-cncm/libmusicxml/blob/lilypond/src/lilypond/musicxml2lilypond.cpp

JM

Le 27 mai 2020 à 15:10, Arshia Cont notifications@github.com a écrit :

@dfober https://github.com/dfober This is now PullRequest #41 https://github.com/grame-cncm/libmusicxml/pull/41 including this fix and lots of others. It is in production on our services and tested (on OSX/iOS). Let me know if there's any problem.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grame-cncm/libmusicxml/issues/40#issuecomment-634648651, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRJX6R7CD2LXLA5N4LTDLDRTUGK5ANCNFSM4NKA4S6A.