Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

BUG in overlap mode?

Developers
Andrés
2012-05-10
2013-05-08
  • Andrés
    Andrés
    2012-05-10

    A lot of years ago (2005 I guess) I discovered a misunderstanding in some code of timidity. Specially in the function "find_voice" (file playmidi.c) with the concept of overlap mode.

    I'm not very very sure about this right now how. In that moment I just analysed the logic of the code and I found a inverted interpretation of some code.

    This code:

    status_check = (opt_overlap_voice_allow) ? (VOICE_OFF | VOICE_SUSTAINED) : 0xff;
    

    Now I don't remember why I get that conclusion. I'm trying to remember.

    So I will begin:
    What mean "overlap mode"?
    It is a mode where accept (as say the timidity manual) pronouncing multiple same notes.

    So if a note is played one time and newly and newly. Just this must sound like a delay line, multiple times overlapping one over other in the time and so.
    The bug is that it just cut off the last note in a moment and next sound the same note (without obey overlaping!). That is because in overlapping it must wait until the note is off but the code turn it off instantly.

    So, the logic (for overlap mode):
    USE THIS VOICE ONLY IF NO NOTE IS SOUNDING HERE

    The portion code:

                    if (voice[i].status != VOICE_FREE && voice[i].channel == ch)
                    {
                            if ((voice[i].note == note && (voice[i].status & status_check))
                                || mono_check
                                || (altassign && find_altassign(altassign, voice[i].note)))
                                    kill_note(i);
                            else if (voice[i].note == note &&
                                     (channel[ch].assign_mode == 0
                                      || (channel[ch].assign_mode == 1
                                          && voice[i].proximate_flag == 0)))
                                    kill_note(i);
                    }
    

    This code kill the note when overlap mode is ON because the first bool expression (status_check) is misunderstand.

    voice_.status & status_check will give us "true" when a note is still sounding because the check is:

    _

    status_check = (opt_overlap_voice_allow) ? (VOICE_OFF | VOICE_SUSTAINED) : 0xff;
    

    _
    That is not correct we must check instead !(VOICE_ON | VOICE_SUSTAINED). So that mean if the voice is NOT ON and if the voice is NOT SUSTAINED it will be killed, but if not, it will continue sounding (overlap).

    The correction:
    _

    status_check = (opt_overlap_voice_allow) ? !(VOICE_ON | VOICE_SUSTAINED) : 0xff;
    

    _
    Maybe we can use too: (VOICE_OFF | !VOICE_SUSTAINED) because maybe the guys of timidity just use VOICE_OFF as a reference to find the voice stopped. But my first code correction sill match that.
    As you can see 0xff match all (if overlap mode is off) so I'm not misunderstanding the thing.

    But wait, there are some misconception in the rest of the code too. So I assume there are some other bugs still to solve.

    What is "mono_check"?
    This:
    _

    mono_check = channel[ch].mono;
    

    _
    Why when the channel is "mono" it must kill the note? I don't understand that. A mono channel cannot overlap at all?? I guess it is a guessing (from developers) for a user trying to reduce CPU usage (so the user will use mono output instead of stereo expecting a CPU usage reduction). Overlapping a lot of notes will use too much CPU. I guess that is because mono_check is used to kill notes too (two birds in a shot). But of course that is a misconception too because using mono or stereo is not related with overlap. Overlap must occur in a voice if it is mono or stereo.

    Anyway, you are free to modify it for yourself and recompile because without overlapping working you will notice what the sustained notes will be turned off instantly and not continue sounding in background to the end (sustaining and decaying notes) as it supposes to be.

    Of course I tested it and the difference is noticeable._

     
  • Yair K.
    Yair K.
    2012-05-13

    This sounds very interesting. I strongly suggest you post this to timidity-help list (the developers don't read this forum as far as I know), and maybe someone who knows the code will step up.

     
  • Yair K.
    Yair K.
    2012-05-13

    Sorry, I meant the 'timidity-talk' list.

     
  • Andrés
    Andrés
    2012-08-10

    Are there still people developing it? Or what they are doing? Because I don't see more versions from some years.

    Anyway if you want publish my message there. I don't know how to do that and I don't have the time. I decided to publish it here because I just noted that the people need to know about that. But I did a big effort posting that.

     
  • SATO Kentaro
    SATO Kentaro
    2012-08-10

    Thank you for the comment.
    But you are looking at the outdated source code.

    status_check = (opt_overlap_voice_allow) ? !(VOICE_ON | VOICE_SUSTAINED) : 0xff;
    

    As status_check is the bit mask, ! operator is not right here.

    > so the user will use mono output instead of stereo expecting a CPU usage reduction
    It has nothing to do with output channels. Check out how the member are set.