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

MIDI export doesn't handle B&B tracks correctly #2384

Closed
grejppi opened this issue Sep 27, 2015 · 68 comments
Closed

MIDI export doesn't handle B&B tracks correctly #2384

grejppi opened this issue Sep 27, 2015 · 68 comments
Assignees
Milestone

Comments

@grejppi
Copy link
Contributor

grejppi commented Sep 27, 2015

Every B&B pattern in the project is placed in the resulting MIDI file in sequence, with a new pattern starting on each bar.

This may be better explained in pictures. A song like this:

kuvakaappaus 2015-09-27 21 05 39

gets exported as if it was like this:

kuvakaappaus 2015-09-27 21 07 26

(cc @mohamed--abdel-maksoud)

@grejppi grejppi added this to the 1.2.0 milestone Sep 27, 2015
@mohamed--abdel-maksoud
Copy link
Contributor

Hi @grejppi , thanks for reporting the bug. Could you attach a sample affected lmms project?

@grejppi
Copy link
Contributor Author

grejppi commented Sep 28, 2015

@mohamed--abdel-maksoud: The project used in the screenshots is CoolSongs/Greippi-ardudar.mmpz.

@mohamed--abdel-maksoud
Copy link
Contributor

OK, I will look into the bug some time next week (unless someone else is working on it already)

@M374LX
Copy link
Contributor

M374LX commented Oct 2, 2015

More information:

The bug does not seem to affect only the MIDI exporter. Problems can be seen even when saving an uncompressed (*.mmp) project file.

Even if the project has more than one BB track, all notes are saved in the first BB track.

@softrabbit
Copy link
Member

The bug does not seem to affect only the MIDI exporter. Problems can be seen even when saving an uncompressed (*.mmp) project file.
Even if the project has more than one BB track, all notes are saved in the first BB track.

What? I get notes in all BB tracks, like this:

  • Open LMMS
  • Load CoolSongs/Greippi-ardudar.mmpz
  • Save /tmp/Greippi-ardudar.mmp
  • Close LMMS
  • Open LMMS
  • Load /tmp/Greippi-ardudar.mmp

(and it sounds reasonably like music, too 😃 )

Using master, up to date: de7d83d

@M374LX
Copy link
Contributor

M374LX commented Oct 2, 2015

What? I get notes in all BB tracks, like this:

Open LMMS
Load CoolSongs/Greippi-ardudar.mmpz
Save /tmp/Greippi-ardudar.mmp
Close LMMS
Open LMMS
Load /tmp/Greippi-ardudar.mmp

(and it sounds reasonably like music, too 😃 )

Everything looks fine when the file is opened on LMMS. Nevertheless, by opening the uncompressed file on a plain text editor, one can see all notes saved in the first BB track.

@softrabbit
Copy link
Member

Nevertheless, by opening the uncompressed file on a plain text editor, one can see all notes saved in the first BB track.

Oh, now I see what you mean... I tried an lmms -d on the original file shipped with LMMS and looked at the XML, and it has the same configuration, the trackcontainer for the BB editor is in the first bbtrack and the following tracks have an empty bbtrackelement and a bunch of bbtcos that are matched to patterns in some non-obvious way. Can't say I like it, but it seems to work as intended for saving and loading files.

It does explain why the MIDI export fails, though. Looks like the export reads the XML and creates the MIDI from that, instead of reading straight from the song. And it assumes any pattern seen belongs straight to the closest enclosing track or something like that.

@musikBear
Copy link

Good find! 👍
! Obviously !
Here anyone who tackles this need to be very careful. This could be a backward compatibility issue.
-sorry if thats all too obvious..

In fact ..
The prove-of-concept would be the following modus o.

  • Create a project entirely with soundfonts of the type generalMIDI
  • Save this project as proj0
  • Export the project to MIDI
  • close project
  • open a new empty project
  • import the exported MIDI-file from proj0
  • change all instruments back to the orr. instrument choices in generalMIDI (unless sub-instrument choices in the SF also should be preserved(?)
  • play project
  • this should now be 100% identical in replay with proj0. If it isent, something is wrong

@tresf
Copy link
Member

tresf commented Dec 7, 2015

@mohamed--abdel-maksoud can you please address the issues noted above so that we can make this feature available in the next release? If not, we be forced to keep this feature disabled.

@Umcaruje
Copy link
Member

Marking this as critical, since this needs to be addressed for 1.2, or we'll need to disable the MIDI Export function.

Also, when a midi file is exported, there is no visual confirmation that the job has been done (a notification bubble would be sufficient for that).

Also there seems to be some debugging info left, since I get my terminal spammed with

06 c8 1c 
06 c8 1c 
06 c8 1c 
...

or similar when I export midi.

ping @mohamed--abdel-maksoud

@mohamed--abdel-maksoud
Copy link
Contributor

Hi,
Terribly sorry for being late on this issue, I've been relocating and got many things to do.
When is v1.2 scheduled?

@tresf
Copy link
Member

tresf commented Jan 26, 2016

When is v1.2 scheduled?

We'd like to get a beta release out next month (Feb), since we're wrapping up most critical issues now.

@mohamed--abdel-maksoud
Copy link
Contributor

OK, I'll look at the issues from tomorrow. Hopefully they'll be fixed in time.

@musikBear
Copy link

or we'll need to disable the MIDI Export function.

I am not sure if i have a updated midi-export binary, (zonkmaschines distro) I know it is not merged! But if i try to re-import an lmms-exported midi, it fails completely. Everything is inserted in one track, and timing also seems to be off

@DeRobyJ
Copy link
Contributor

DeRobyJ commented Feb 3, 2016

In the beta, we may just alert the user that midi exporting won't work well with BB tracks...

@Umcaruje
Copy link
Member

Umcaruje commented Feb 3, 2016

In the beta, we may just alert the user that midi exporting won't work well with BB tracks...

No. That's not fixing the problem, its sweeping it under a rug.

@musikBear
Copy link

No. That's not fixing the problem, its sweeping it under a rug.

Yes 👍 Imo midi export could / should be postponed to 1,2,1. Even though it has been mentioned now and then, no one has promised midi-export. 1.2 will still be a bundle of goodies!

@DeRobyJ
Copy link
Contributor

DeRobyJ commented Feb 5, 2016

But if we make it accessible we can test it even more and discover a lot more bugs that may lead us to the real problem...
I already tried 3 days to compile lmms, both in linux and in windows, because I want to test it and I couldn't make it, and I don't have time to search my problem right now.

I'm not saying to put it in 1.2 (or RC), I was talking about the beta.

@grejppi
Copy link
Contributor Author

grejppi commented Feb 5, 2016

Quite frankly the real problem is that MIDI data is not rendered in the same way as with audio export... The current method blindly looks for instrument patterns in the project file and converts them into MIDI notes at appropriate positions. Instrument patterns inside the B&B editor also have a position property, but it is used only to determine which B&B pattern it belongs to.

It's impossible to tell a B&B instrument pattern from a pattern in the song editor just by looking at the individual element without context.

@Fastigium
Copy link
Contributor

A most interesting issue... Some observations:

  1. The MIDI export plugin does not use the project file. Instead, it converts Track objects to XML on-the-fly and then parses that XML. An advantage of this approach is that changes in the internal structure of Track objects do not influence MIDI exportation as long as the XML representation remains compatible. However, I have no idea how fixed (or even defined) the XML representation of a Track object is in LMMS.
  2. The primary bug here is caused by the way the MIDI export code handles tracks. To refine what @grejppi already stated, it processes instrument tracks only. Normally, this would result in B&B patterns not being processed at all, since B&B instrument tracks are located in a separate container, but the export plugin is handed a specially-constructed TrackList containing first the Song tracks and then the tracks from the BBTrackContainer. However, the instrument tracks in the BBTrackContainer do not have data in their XML representation on where in the song their patterns are used.
  3. The reason that re-importing an exported MIDI ends up with all notes on a single track appears to be that the MIDI import plugin is optimized for GM-compatible MIDI files. It splits the notes not by MIDI track, but by the program or patch number set by MIDI events. The MIDI export plugin does not set program or patch numbers, and therefore the import plugin does not split into multiple LMMS tracks.

Changing the way the track-to-XML-conversion is handled right now could fix the B&B exporting bug (the data needed to process B&B tracks correctly is there, it's just not used yet). However, I wonder if it would be a better idea to get rid of the XML conversion entirely and just work with LMMS's objects themselves. How stable is that structure expected to be compared to the XML representation?

Fixing the export/re-import bug seems complicated enough to warrant its own issue, so I propose to redirect further discussion on that elsewhere.

@mohamed--abdel-maksoud
Copy link
Contributor

insightful remarks in the last two posts.
I could start on one of two directions:

  1. keep the project->xml->midi workflow and only adapt it to incur less bugs, or
  2. rework the whole plugin to work in a similar way as the audio export probably.
    What do you think?

@Fastigium
Copy link
Contributor

After giving more thought to how this would be implemented, I realize that there is another issue we need to address first: how are B&B tracks expected to be exported? MIDI has no support for defining patterns and repeating them AFAIK. All we can render are notes and events at certain times, possibly split out in tracks. Because a B&B track can play notes to multiple instruments, putting all notes in a single MIDI track means a loss of information. For drum patterns, this could be solved by putting the notes for each instrument on a different tone, but B&B patterns can contain melodies as well, which wouldn't work well with that approach.

A different approach would be to export the notes of the song as a whole. That would mean processing B&B tracks into the resulting notes to their instruments, and then exporting those instruments as MIDI tracks. Importing such a MIDI file, assigning the right instruments to the tracks and playing would result in a faithful reproduction of the whole song. However, the individual B&B patterns would be lost and require manual copying and pasting of notes to be restored.

FL Studio "fixes" this problem by not allowing to export the whole song to MIDI at all. It only exports the individual patterns and leaves you to redo the arrangement in the program in which you import it.

LMMS, however, allows notes that exist only in the song editor, meaning that FL studio's approach won't work. Perhaps the best solution for LMMS would be to export the notes of the song as a whole by default, and allow exporting individual B&B patterns for those who want to use those in a different program. Thoughts?

@DeRobyJ
Copy link
Contributor

DeRobyJ commented Feb 21, 2016

Let's try to understand the users' needs.

Exporting a single instrument track is already a good thing, as most of the times midi is used to copy notes between programs.
Who'll be exporting a whole song?
It's not probably someone trying to re-open it in LMMS. Project files are the best solution and you won't need midi to pass a song from a project to another (you may actually export a single track to copy it between projects)

Most likely, it would be used to create a midi song intended for listening or to pass the project to another program.

In both cases, keeping the bb patterns is not needed (or trying to leave clues for LMMS to recreate them).

So the only problem I see here is passing from bb editor tracks to instruments and drumkit midi tracks, as fastigium said.

Since automatically detecting the kind of instrument isn't an option, we will need the user to do the job of joining all the drum samples into a single track using a general midi soundfont.

In the end, if the the midi file is made for listening, the user should edit it as he intends it to be heard, full of sf2 tracks, just as an imported midi.

If it is not, then keeping the instruments is probably not important.
If the user needs to copy a drum pattern across programs, well, he can either export the singles samples tracks or create a general midi formatted track (if the receiving program works better with that).

Note that general midi doesn't have single samples patches, it only has drumkits.

@musikBear
Copy link

@DeRobyJ
I agree
Imo the situation is more a teaching/ instructing 'job' aka a page in the wiki, where it is explained how notes from all individual BB-editor-tracks, need to be, not only manually copied to either the zasfx preset drums or a 'SF2 generalUser128-patch', but also manually extended over the whole track manually by the user.

Imo it would even be acceptable to mint the user to exchange all instruments in the project manually for SF2generalUser presets, and manually select the patch for each instrument,
before an midi-export. That would also ensure that the exported track, sounds like the user prefer.

Then LMMS should then export a midi-file with the content.

So ok, a midi export is not a one-click-wonder.. So what?

One issue that still could be a plague is the inserts on the presets FX-page. If the original LMMS projects uses VST-effects there, then what?
Make use of the SF2midi-patch own effects? How? I can only see manual by user as an answer.

schedule imo 1.2.1 or later

@softrabbit
Copy link
Member

I tried it out and exporting instrument tracks gives me all tracks together in one MIDI track when I import it back into LMMS(Track 0, pic). I haven't had time to test it enough though.

@zonkmachine, how does that MIDI file behave when importing in something else? That would probably be the most common case. And it could be that the import code is less than perfect...

@zonkmachine
Copy link
Member

@zonkmachine, how does that MIDI file behave when importing in something else? That would probably be the most common case. And it could be that the import code is less than perfect...

Indeed, if I open the same file in MusE it looks correct.

@zonkmachine
Copy link
Member

I think the Javascript file converter is very promising and I could take inspiration from it handling b&b

Did you look into this any further? I tried the online converter just briefly and it made a correct export of the demo project shorties/Crunk(Demo).mmp.

@PhysSong
Copy link
Member

PhysSong commented Jul 4, 2017

I'm working on it because I need MIDI export feature now. @DeRobyJ's approach seems good for me.

I found notes with length -192 inside B&B patterns. Handling these notes is important because it might cause underflow or other bugs.

@PhysSong
Copy link
Member

PhysSong commented Jul 4, 2017

Also there seems to be some debugging info left

It is in Event::writeToBuffer(), file MidiFile.hpp. When fixing is finished, marking that as a comment seems fine.

@zonkmachine
Copy link
Member

A bit off topic... MidiFile.hpp lacks a license. It references one but the actual license text isn't there.

 * Created:     2008/04/17
 * Copyright:   (c) 2009 Mark Conway Wirt
 * License:     Please see License.txt for the terms under which this
 *              software is distributed.

I did a search and found:
http://metadata.ftp-master.debian.org/changelogs/main/p/python-pyknon/unstable_copyright

Files: pyknon/MidiFile.py
Copyright: 2009 Mark Conway Wirt <(emergentmusics) at (gmail . com)>
License: MIT
 Permission is hereby granted, free of charge, to any person obtaining
 a copy of this software and associated documentation files (the
 "Software"), to deal in the Software without restriction, including
 without limitation the rights to use, copy, modify, merge, publish,
 distribute, sublicense, and/or sell copies of the Software, and to
 permit persons to whom the Software is furnished to do so, subject to
 the following conditions:
 .
 The above copyright notice and this permission notice shall be
 included in all copies or substantial portions of the Software.
 .
 THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
 EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
 MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
 NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
 LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
 OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

I just assume that if it's got 'Debian' stuck to it, the license has been reviewed thoroughly.
@mohamed--abdel-maksoud Can we just stick this license on top of MidiFile.hpp instead of the reference?

@zonkmachine
Copy link
Member

It is in Event::writeToBuffer(), file MidiFile.hpp. When fixing is finished, marking that as a comment seems fine.

printf() here. I think so too but I would add the excplicit braces {} around the printf() statement to keep it visually apart from the assignments that follows. So it would look something like:

//for( int i=0; i<3; i++ ) { printf("%02x ", fourbytes[i+1]); }
//printf("\n");

Or remove it completely...

@mohamed--abdel-maksoud
Copy link
Contributor

@zonkmachine Sure, I can do the modifications (immediately?)
fyi I'm pausing the work on this feature since @PhysSong is already working on it

@zonkmachine
Copy link
Member

@zonkmachine Sure, I can do the modifications (immediately?)

Right now is fine! ;-)

fyi I'm pausing the work on this feature since @PhysSong is already working on it

👍

@tresf
Copy link
Member

tresf commented Jul 8, 2017

The rewrite can and should be relicensed GPL2. The MIT license is compatible and leaving the original copyright should fulfill the needs of the original MIT.

Please edit the file to use the standard GPL2 heading and change the python reference to mention the original license by name only so that others wishing to use the code under MIT specifically can go back to it as they wish.

Gaining mohammed's permission to use GPL2 is a nice gesture, but it's irrelevant. We can legally use this file MIT, therefore we can use it GPL2.

@PhysSong
Copy link
Member

PhysSong commented Jul 8, 2017

I found another bug of MIDI export: If two notes have the same pitch and one note starts at the position where another note ends, sometimes some exported notes have wrong lengths.

@tresf
Copy link
Member

tresf commented Jul 8, 2017

@zonkmachine Sure, I can do the modifications

Sorry,missed this part. @mohamed--abdel-maksoud was the original intent to keep MIT, GPL2+ or use another license. We must operate under the assumption that any code not specified is GPL2+.

@mohamed--abdel-maksoud
Copy link
Contributor

@tresf I ported the code for MIDI output from this MIT-licensed project (https://code.google.com/archive/p/midiutil/). As I'm not an expert in OS licenses, I played safe and kept the license of the original code: MIT.
Is it proper to switch it to GPL?

@tresf
Copy link
Member

tresf commented Jul 8, 2017

Is it proper to switch it to GPL?

It's proper because it's less confusing to those who download our code. They're compatible, yes. GPL2+ would be advantageous since this code was written solely for LMMS and + gives us a path to bring this to GPL3, assuming VST3 eventually pushes us there. Ironically GPL2 and GPL3 are not compatible, so the + is critical.

@grejppi
Copy link
Contributor Author

grejppi commented Jul 8, 2017

If it is a port from an existing project I would find it fair to the original author to keep the license they chose 🤔

It wouldn't make LMMS any less GPL'd anyway.

@tresf
Copy link
Member

tresf commented Jul 8, 2017

If it is a port from an existing project I would find it fair to the original author to keep the license they chose 🤔

There's no "fairness" argument when relicensing, just what's allowed. That's sort of the point of a permissive license to begin with.

@tresf
Copy link
Member

tresf commented Jul 8, 2017

It wouldn't make LMMS any less GPL'd anyway.

Exactly, so we should call a spade a spade.

@PhysSong
Copy link
Member

Is the 1.2.0 milestone for this issue appropriate? I think I can fix it in about a week, so the time doesn't matter.
The main problem is: should 1.2.0 include midi export feature if it is fixed?

@PhysSong PhysSong self-assigned this Jul 18, 2017
@Umcaruje
Copy link
Member

The main problem is: should 1.2.0 include midi export feature if it is fixed?

Yes, it was already a feature in 1.2-RC1

@zonkmachine
Copy link
Member

Yes I think so too. Apart from being a milestone it's a feature that is frequently requested in the forum.

@jnm2
Copy link

jnm2 commented Jul 18, 2017

Yes please, I'm waiting for it.

@musikBear
Copy link

If the points are explored, the 'need' is not so obvious:

  • There is already an exporter on a website
  • Does the implementation take away important focus on things that holds the release back?
  • Is a week a realistic schedule?

This was referenced Jul 26, 2017
@mendomusic
Copy link

mendomusic commented Sep 22, 2017

Thanks to everyone who made this happen!
Checked it with the latest git master & it worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests