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

Audiveris import #5692

Merged
merged 1 commit into from Mar 18, 2020
Merged

Conversation

igorkorsukov
Copy link
Contributor

@igorkorsukov igorkorsukov commented Feb 11, 2020

Resolves: (direct link to the issue)

Optical music recognition by integration with audiveris app

Use "x" letter to fill the checkboxes below like [x]

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
    [N/A] I created the test (mtest, vtest, script test) to verify the changes I made

As for how it works:

  • The file pdf, png, jpg is transferred to the OMR engine (used Audiveris)
  • OMR engine can be either on the server, i.e. the file is sent over the network to the server. This is the default behavior.
  • Or OMR engine can be local, for this you need to in the Preferences -> Import check to use local. Then the editor will download the portable build of OMR engine and its dependencies, install it in the Extensions folder, and use it.
  • From OMR engine we get MusicXML and the OMR file, which contains information about recognized and unrecognized elements. Both files are zipped in one zip archive.
  • The MusicXML is imported into MU Editor by its function, and I read OMR file, and draw under score.
  • The main problem that I solved was to compare the data from OMR with the elements of score, because they are on a different scale, different size, different position, etc.
  • Also, one of the tasks is to remove excess garbage, there are some features of the operation of Audiveris.
  • As a result, I mark with the colors those elements that Audiveris recognized, and those that did not recognize (in the OMR file are marked as not recognized)
  • A legend is shown to the user what the colors mean (you can hide it by clicking on it)
  • In the View menu, you can hide recognized elements to see problematic ones, or hide all elements to see the final result
  • You can start editing the score, correct what needs to be fixed, then save the recognition project (.msmr file), and later open it and continue working.

I think it is possible to improve the matching of data from OMR and Score, this is the work of the future.

avsomr/CMakeLists.txt Outdated Show resolved Hide resolved
avsomr/avslog.hpp Outdated Show resolved Hide resolved
libmscore/score.h Outdated Show resolved Hide resolved
mscore/file.cpp Outdated Show resolved Hide resolved
mscore/scoreview.cpp Outdated Show resolved Hide resolved
@Harmoniker1
Copy link
Contributor

@Jojo-Schmitz Why not create those review comments first and submit them altogether?

@Harmoniker1
Copy link
Contributor

@igorkorsukov And yes, there're some pretty serious formatting (especially indentation and brace usage) issues, please read MuseScore coding rules.

avsomr/CMakeLists.txt Outdated Show resolved Hide resolved
avsomr/avsomrdrawer.h Outdated Show resolved Hide resolved
avsomr/avslog.h Outdated Show resolved Hide resolved
avsomr/avslog.h Outdated Show resolved Hide resolved
avsomr/avsimport.cpp Outdated Show resolved Hide resolved
avsomr/avsimport.cpp Outdated Show resolved Hide resolved
avsomr/avsomr.h Outdated Show resolved Hide resolved
avsomr/avsomr.cpp Outdated Show resolved Hide resolved
avsomr/avsomrreader.h Outdated Show resolved Hide resolved
libmscore/score.cpp Outdated Show resolved Hide resolved
@Harmoniker1
Copy link
Contributor

@igorkorsukov All function definitions in .cpp files and all class definitions in .h files should have comments simply repeating the names in front. Like:

//---------------------------------------------------------
//   symbol
//---------------------------------------------------------

You can add other explanatory things between the two long lines if you like.

mscore/scoreview.cpp Outdated Show resolved Hide resolved
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Feb 11, 2020

Under which name did you sign the CLA?
AFAIK that is needed even for staff members.

@igorkorsukov
Copy link
Contributor Author

igorkorsukov commented Feb 11, 2020

You'd need to sing the CLA...
And of course fix those error that lead to Travis CI and AppVeyor to not compile.

Sorry, I did not know that C ++ 17 is not supported, fixed it.

Does it build on your machine? Which platform?

My Linux platform, gcc version 7.4.0

@igorkorsukov igorkorsukov changed the title Audiveris import [WIP] Audiveris import Feb 11, 2020
CMakeLists.txt Outdated Show resolved Hide resolved
avsomr/avsomrreader.h Outdated Show resolved Hide resolved
avsomr/avsomrreader.h Outdated Show resolved Hide resolved
@igorkorsukov igorkorsukov changed the title Audiveris import [WIP] Audiveris import Mar 12, 2020
@igorkorsukov
Copy link
Contributor Author

@Tantacrul started working on this feature, the behavior and design will be changed, so again WIP

@igorkorsukov igorkorsukov force-pushed the audiveris_import branch 2 times, most recently from 11d3950 to b6db992 Compare March 12, 2020 11:59
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Mar 12, 2020

Better use git pull --rebase musescore master (or upstream, if that's what it is called in your repo), rather than git merge

@igorkorsukov igorkorsukov force-pushed the audiveris_import branch 2 times, most recently from 1e38367 to 7d96f1e Compare March 12, 2020 12:33
@igorkorsukov
Copy link
Contributor Author

Better use git pull --rebase musescore master (or upstream, if that's what it is called in your repo), rather than git merge

Yes, thank you, I got a little confused with git today, but I put everything in order in the end.
Sorry for spam.

@igorkorsukov
Copy link
Contributor Author

Since most of the code is separate and does not affect the main code much, I'm going to merge to master the current PR with this experimental function, but it will be disabled by default (at the compilation stage). So that subsequent reviews and discussions of changes (there will be changes in UI/UX), were more effective.

@MarcSabatella
Copy link
Contributor

If we're pretty sure the code will be used but maybe with a different UI, Id personally prefer to see it enabled so people can test the functionality itself. We also have an "experimental" flag ("-e" command line option) that things like this are sometimes hidden behind.

@Tantacrul
Copy link
Contributor

t

If we're pretty sure the code will be used but maybe with a different UI, Id personally prefer to see it enabled so people can test the functionality itself. We also have an "experimental" flag ("-e" command line option) that things like this are sometimes hidden behind.

For what it's worth - I'm in the middle of putting together a different design for this idea. Uses some of this stuff but is quite different. I don't know how that affects your decisions here but I thought I'd mention it.

@igorkorsukov igorkorsukov changed the title [WIP] Audiveris import Audiveris import Mar 16, 2020
fluid/voice.cpp Outdated Show resolved Hide resolved
mscore/prefsdialog.cpp Outdated Show resolved Hide resolved
mscore/prefsdialog.cpp Outdated Show resolved Hide resolved
@Jojo-Schmitz
Copy link
Contributor

rebase needed

.gitignore Outdated Show resolved Hide resolved
mscore/file.cpp Outdated Show resolved Hide resolved
libmscore/score.h Outdated Show resolved Hide resolved
avsomr/msmrwriter.cpp Outdated Show resolved Hide resolved
mscore/prefsdialog.ui Outdated Show resolved Hide resolved
@anatoly-os anatoly-os merged commit 475de86 into musescore:master Mar 18, 2020
@RobFog
Copy link

RobFog commented Mar 18, 2020

Does the merged code implement @Tantacrul's design?

@igorkorsukov
Copy link
Contributor Author

Does the merged code implement @Tantacrul's design?

No, we are waiting for the final design. Now this feature is merged with disabled by default at the build level.

@igorkorsukov igorkorsukov deleted the audiveris_import branch October 27, 2020 08:53
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