LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
8.16k stars 1.01k forks source link

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

Closed grejppi closed 7 years ago

grejppi commented 9 years ago

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)

mohamed--abdel-maksoud commented 9 years ago

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

grejppi commented 9 years ago

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

mohamed--abdel-maksoud commented 9 years ago

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

M374LX commented 9 years ago

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 commented 9 years ago

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:

(and it sounds reasonably like music, too :smiley: )

Using master, up to date: https://github.com/LMMS/lmms/commit/de7d83d15897407b6300e2e3475ce6f7c62903a2

M374LX commented 9 years ago

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 :smiley: )

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 commented 9 years ago

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 commented 9 years ago

Good find! :+1: ! 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.

tresf commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

musikBear commented 8 years ago

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 commented 8 years ago

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

Umcaruje commented 8 years ago

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 commented 8 years ago

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

Yes :+1: 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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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

DeRobyJ commented 8 years ago

I'm thinking that this will lead to having a copy of the project , a "midi-exportable" version of it.

We may actually make use of the new MISC tab of the instruments and create a more user-friendly system. Midi export options, radio button between:

OR

Then, while exporting, the program uses those informations, and creates a single drumkit track.

For reference: https://en.wikipedia.org/wiki/General_MIDI

Umcaruje commented 8 years ago

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.

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

Is this how you would solve something in real life?

"Hey, I know that faucet has a broken pipe and only cold water comes out of it, but hey you don't really need hot water anyways. I'll make you a tutorial how to heat the water with an iron, if you really need it"

This issue needs to be resolved before we release 1.2. Its a bug, its not a feature, and MIDI export shouldn't work like that. The BB Editor is not only for drums and it can be used to hold melodies, samples or anything you want. There are people that use the B&B editor to make entire songs (as @grejppi) and they need to be able to get their songs properly exported.

Fastigium commented 8 years ago

I'm not sure we want to implement exporting MIDI files meant for listening. MIDI sequencers are much better suited for that. For it to work in LMMS, the user would have to adhere to very strict guidelines, or the exporter would have to make some magic guesses. Plus it would be a lot of work to implement.

Exporting MIDI files to carry notes/arrangements to other programs seems much more sensible to me. The current implementation does not do that correctly for B&B tracks. If we fix that, the export function should be useful enough to be in 1.2. The only question is: how do we translate the versatility of LMMS's B&B tracks/patterns into MIDI tracks in such a way that it makes sense?

DeRobyJ commented 8 years ago

Ideally, a bbeditor is a single event that calls different pianorolls events.

I see a bbeditor block as a group of piano rolls, and we should treat it like that.

The only difference is that different bb tracks do not mean different instruments, they are just notes on the same n tracks of the bbeditor.

In my opinion, we should let the user decide the midi instrument that should be played and export a single track per instrument (with the exception of drumkit as I explained before). If it is practically possible, then we can export midis meant for listening.

edit: I'm really sorry for my English, I'm sure I didn't explain my idea well, so i couldn't help myself from doing two pictures of how I think we can achieve a good midi exporting. I want to stress that this isn't meant to offend you, I'm sure some of you could easily understand what i said or had a similar idea in mind, I just wanted to shere these, hoping they are clear.

So, before starting the exporting process, LMMS needs to "convert" BB tracks like this: midi export 1 I believe this can be easily done with a pseudo-recording process. The program creates these fictional instrument tracks and records the note events triggered by all the different bb blocks

Then, if the user has made input in a new window of the MISC tab of every track, the program can assign each instrument to the appropriate track. I repeat myself: in each lmms track, the user will be able to chose between

In the first choice, the user has to input the kind of instrument. A piano, a violin, pizzicato strings, eventually a drumkit, if he used a GM-compatible drumkit. In the second choice, the user just decides what kind of percussion is that.

The program then creates the MIDI tracks like this: midi export 2

musikBear commented 8 years ago

@Umcaruje

The BB Editor is not only for drums and it can be used to hold melodies, samples or anything you want.

! That is true. That does however add a huge chunck of complexity to the already difficult area.

"Hey, I know that faucet has a broken pipe and only cold water comes out of it, but hey you don't really need hot water anyways. I'll make you a tutorial how to heat the water with an iron, if you really need it"

Ahrr :) funny as it is, but is it really a fair comparison ?

Is this how you would solve something in real life?

nahh ironing water, would not be on my repertoire :p, but you surely could find me making a very 'alternative' solution to a broken pipe Back to topic

This issue needs to be resolved before we release 1.2. Its a bug, its not a feature, and MIDI export shouldn't work like that.

Here we disagree. Imo midi-export is not a necessity for 1.2. It would be fully acceptable to release 1.2 without any midi-export at all. Simply postpone this new feature for 1.2.x, and perhaps then even be able to do like @DeRobyJ outline so nicely( :+1: but i fear complexity is too high in the current BBeditor to conform to the model). So, *imo the new feature midi-export could wait.

DeRobyJ commented 8 years ago

Mmm I'm actually with Umcaruje... Thing is, we don't need to make 1.2.0 quickly. It would be appreciated, but I prefer another delay than an incomplete first stable.

On the other side, we'd be delaying other features. However, some of them can be improved in the meantime, and since we are going to release a new default theme, we may do UI changes in some plugins.

softrabbit commented 8 years ago

My view on how this could work, using mostly existing functionality. Probably more optimized for data interchange than making GM files for listening.

The user could be warned that some tracks are unassigned and presented with the choices "cancel" and "auto assign" when exporting.

I'm with @musikBear here, this could well wait until 1.2.x. Or then a third option could be chosen, release the feature in a not quite 100% working state but not too easily accessible; maybe require some hoop to be jumped (edit lmmsrc.xml?) to enable it? That's not exactly unheard of.

Umcaruje commented 8 years ago

@mohamed--abdel-maksoud is there any progress on this issue?

mohamed--abdel-maksoud commented 8 years ago

@Umcaruje no progress so far. From the discussions here, I cannot see a clear direction of how this should be handled. I don't have enough (musical) experience with BB tracks to judge the suggestions here. Is the input from @softrabbi the way to go?

Umcaruje commented 8 years ago

@mohamed--abdel-maksoud sorry for the late responce, yes @softrabbit's proposal sounds right.

Something about this needs to be done. We could make it a hidden, experimental feature in lmmsrc.xml and then proceed with 1.2, and when the feature gets fixed, we enable by default in a patch version, as @softrabbit suggested. I like that idea.

jasp00 commented 8 years ago

@mohamed--abdel-maksoud, I do not believe the export system is ready. Having only the model available without functions to interpret the model means to virtually write a second core. Automation and pattern lengths need interpretation.

My guess to solve the export is to add a hook to InstrumentTrack::processOutEvent(), play the song, and process MIDI events.

grejppi commented 8 years ago

So, should midi export be disabled or left as is, or possibly even fixed for RC2?

grejppi commented 8 years ago

I did some investigation and may have found a way to make b&b instruments behave as sing editor instruments by modifying the intermediate data to be exported. If I have time I'll try to implement it in the next weeks but I can't promise anything

DeRobyJ commented 8 years ago

Since things hasn't changed, I did my reading of the code. In my opinion, we should try doing the simplest "fix" possible: change this line base_time = n.toElement().attribute("pos", "0").toInt();

Now, I'm not familiar with QDomNode and QDomElement, but the problem is we're taking a "pos" attribute from the wrong element.

Rather than taking the position from the pattern itself, the program should take the BBTCO's pos. This, if I'm correct.

But I have no idea how to access that element. Maybe this isn't the best fix, but can anyone give it a try? Just to tackle the problem a bit? Then we'll decide if we're to fix XML conversion or we're to use lmms objects.

Umcaruje commented 8 years ago

Per #3114 Midi export is now disabled. We can move on with 1.2 release.

DeRobyJ commented 8 years ago

Ok. Even if I strongly think this issue just needs someone with slightly more experience. After thinking a bit about it (that's what I can do for now xD), I believe @grejppi was going in the right direction: we may write bbtracks as instrument tracks in the xml port of the song. Doing it inside the midi export plugin is harder because the process is really procedural.

musikBear commented 7 years ago

Here is news on this issue http://lmms.io/forum/viewtopic.php?f=4&t=6053&p=23191#p23190 (java-script source-code) I must say that i have not done proper testing of the online tool. I think it will be necessary to see a project being exported as midi using the online-tool, and subsequently imported as midi in lmms, without flaws, that to me would be a passed test

zonkmachine commented 7 years ago

Exporting needs more time it seems. No one is working on this so should we bump MIDI export to 1.3?

Edit: the following comment seem to be related to the import function and a separate bug. See discussion below 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. midiscrambled

Here is more info hidden in a closed PR: https://github.com/LMMS/lmms/pull/1686#issuecomment-118434710

musikBear commented 7 years ago

@zonkmachine i believe prosponeing midi-export already has been agreed on? I vote bump to 1.2x

mohamed--abdel-maksoud commented 7 years ago

I have not followed this thread for some time, I think the Javascript file converter is very promising and I could take inspiration from it handling b&b When is the scheduled release date for 1.2?

zonkmachine commented 7 years ago

@mohamed--abdel-maksoud Awesome! I'll do a proper round of testing then.

When is the scheduled release date for 1.2?

There is no set release date but I think the general idea is 'a couple of months' or so. Basically it's released when we're done but we haven't released anything in years so we need to get this out. However... The next release, 1.3, will not be years away so if the pain of coding this becomes unbearable there's no reason to panic... at least not over the release date. There is always reason to panic.

softrabbit commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.