Mindwerks / wildmidi

WildMIDI is a simple software midi player which has a core softsynth library that can be used with other applications.
https://github.com/Mindwerks/wildmidi
Other
203 stars 42 forks source link

event struct needs to retain certain events #80

Closed chrisisonwildcode closed 9 years ago

chrisisonwildcode commented 10 years ago

Nothings broken, until you try to visualise turning the event struct into a midi file. Because all the timings are pre-calculated and converted into sample counts there is no information in the struct that can give you BMP or some other things that may be usefully put in the WM_Info struct such as TimeSignature, KeySignature and more importantly text and lyrics for karaoke :)

If the following events occurs within the files being processed they will also appear in the midi file output:

_A marking of (done) means it has made it into my development branch. _* Note: An event listed here does not mean WildMidi supports it, only that if it exists in the input file then it will be output in the midi file WildMidi is able to produce. However the framework is in place for WildMidi to support events marked as (done).

psi29a commented 10 years ago

KAR text? :)

BTW: _ConvertToMidi exists already in master, you mean re-writing it?

chrisisonwildcode commented 10 years ago

Yeah, re-write it to convert the event struct into a midi file. @sezero said that the direct *ToMidi functions work so may as well reclaim this one for the event struct.

chrisisonwildcode commented 10 years ago

Can I get someone more skilled than me to write a buffer type function like what was done for events but for text strings and return an address of the \0 terminated buffered text.

Example: uint8_t * buffer_text(uint8_t *text, uint32_t text_length)

Otherwise I'll just use malloc() and memcpy() for the text and lyric events.

chrisisonwildcode commented 9 years ago

Updated original post

chrisisonwildcode commented 9 years ago

Here is a question: the remaining Meta events I would like to support and hence be able to be output into midi file by from the event struct are all text events.

This means their format is FF ME LL ?? ?? ?? ... where ME is the Meta event code, and LL is the length of data.

What I would like to do is where the event struct normally stores the data in uint32_t I would like to store the address of a malloc'd chunk of memory ... something like

uint8_t * storetext = NULL; storetext = malloc(LL); memncpy(storetext, data, LL); mdi->events[mdi->event_count-1].event_data.channel = LL; mdi->events[mdi->event_count-1].event_data.data = (uint32_t)storetext;

then later ...

uint8_t * writetext = NULL;

writetext = (uint8_t *)mdi->events[mdi->event_count-1].event_data.data;

/* Remember we stored the length in the channel since the event does not have a channel setting. _/ memcpy((_out),writetext,mdi->events[mdi->event_count-1].event_data.channel);

This would mean I don't have to break the internal format just to add text events.

Please inform me if this is possible, and correct.

psi29a commented 9 years ago

You're re-purposing a variable "channel" to store this data. While you can do it this way, it isn't going to be very obvious unless you make a note of it with a comment block.

Why are you hesitant to break the internal format? Adding an additional field/variable to a struct isn't going to cost us anything, right? We can always realloc these extra "meta" fields if they grow.

psi29a commented 9 years ago

Please correct me if I'm wrong, but couldn't we just add the extra pointer * data members to the struct, set to NULL. The moment that we need them, as in, oh look.. lyrics, we realloc with our 4096 define and keep track of usage. The moment we need more memory, realloc for double so much.

I'm not sure how that is breaking the internal format or am I missing something?

chrisisonwildcode commented 9 years ago

Yep, was just trying to save memory :)

I'll get stuck into this over the next few days and that way we can close off the Kar ticket too :)

psi29a commented 9 years ago

You don't need to worry about memory, pre-allocating 4K and not using all of it is perfectly OK. If you are really worried, just initialize to NULL until you have you fill it and then allocate the 4K, but I think it best to allocate 4K for all the fields you expect to come across even if they aren't used.

I would be worried if you started allocated 512K or more at once. ;)

psi29a commented 9 years ago

Oh... is this for 0.4 or 0.4.1? Right now, it is listed as wishlist.

I would like to wrap up the 0.4 release. :)

chrisisonwildcode commented 9 years ago

make it for 0.4.1 ... lets tidy up what we have ... I still have to do the the txt for midi file format

chrisisonwildcode commented 9 years ago

Oh and try to break what we have before we release it ... prefer to work on planned features after 0.4 release instead of bug fixes :)

chrisisonwildcode commented 9 years ago

All the events listed in the top post are now sudo-supported by wildmidi. This means they will appear in the "m" output midi file, including the text events.

Closing ...