Re: [Audacity-devel] problems with recent MIDI code

[Roger D. <rb...@cs...>]

On 11/11/10 7:26 PM, Gale Andrews wrote:
> | From Roger Dannenberg<rb...@cs...>
> | Thu, 11 Nov 2010 16:44:03 -0500
> | Subject: problems with recent MIDI code
>> It was certainly my understanding and intention too to isolate
>> MIDI_OUT and EXPERIMENTAL_SCOREALIGN changes properly. I know I made and
>> already fixed some mistakes related to which code was enabled by which
>> switches, but I just double-checked and agree there are still some bugs,
>> e.g. it looks like turning off USE_MIDI will cause problems in
>> MixerBoard. In retrospect, I guess I wasn't thinking of the compiler
>> switches as a non-trivial programming task, but I see now that it's
>> easier to make mistakes there than I imagined.
>>       I understand the motivation to back out changes, but I'm worried
>> that reasoning about interactions between sets of changes and conflict
>> resolution might be more difficult than just fixing the problems at
>> hand. Also, while we know the changes at least have some configuration
>> bugs, they are likely to have some real bug fixes, so backing out the
>> changes may "back in" a few bugs.
>>       I would suggest that we (meaning I) go back to a (somewhat painful)
>> technique that Greg Williams an I used, which is to run the C
>> preprocessor on the code before and after changes and use diff to look
>> for changes, e.g. with EXPERIMENTAL flags off. I could identify and
>> either remove or document for discussion any changes outside of the
>> EXPERIMENTAL flags.
>>       I think this would be just as (or more) effective at fixing
>> problems, and ultimately it seems like it must be done anyway unless in
>> the near future we just take the flags out (I'm not going to advocate
>> for that). I have some time now (and the egg-on-the-face factor is at
>> work too) to do a systematic pass to identify and review all
>> non-EXPERIMENTAL changes I've made. You have more experience with
>> versioning and code management, so you might have other ideas or better
>> ways to do this. In any case, I'll defer to your decision.
> Considering these problems are not in a released build, and depending on
> a likely time scale for what Roger suggests, I'm tempted to think going
> forwards from the current position rather than backwards and starting
> over could be a better approach. Of course that's only a pragmatic
> rather than a technical response.
>
> I'd also like to raise a question I raised on -quality, in case it makes a
> solution easier or not - do we need USE_MIDI on for 2.0? I note you
> say turning USE_MIDI off could cause some problems in Mixer Board,
> but does USE_MIDI switch on the extra MIDI features that 1.2 doesn't
> have (MIDI "cut and paste editing" and export) or also control MIDI
> import?
The recent MIDI features are enabled/disabled by USE_MIDI along with 
Note Tracks, but switching it off will cause at least some problems, so 
there's some work to be done regardless.
> Neither the 1.2 or 1.3.13 current MIDI features are going to seriously
> impress MIDI users, IMO. In fact I think the MIDI "features" in 1.3.13
> still look very unfinished, and will always do until we at least get stable
> MIDI playback.
>
>
>
> Gale
>
>
>
>
>
>> On 11/10/10 8:55 PM, Vaughan Johnson wrote:
>>> Roger, when I recently said it was okay to add the new MIDI out stuff,
>>> it was with the understanding that you were going to do it all under
>>> EXPERIMENTAL_MIDI_OUT and not threaten our code stability with that
>>> turned off.
>>>
>>> As you know, we are desperately trying to wrap up the P2 bugs and get
>>> Audacity 2.0 out. EXPERIMENTAL_MIDI_OUT will be off for 2.0, but
>>> USE_MIDI will be on.
>>>
>>> I've worked on some crash-causing bugs recently that were introduced by
>>> your changes. In fixing bug 250
>>> (http://bugzilla.audacityteam.org/show_bug.cgi?id=250) and looking into
>>> bugs described in recent messages on the "Mixer Board issues" thread on
>>> the audacity-quality list, I've found that you did some of these changes
>>> under EXPERIMENTAL_MIDI_OUT, but a lot under USE_MIDI, and even more
>>> with no compiler flag at all. For instance, in MixerBoard, there are
>>> numerous special cases for NoteTrack that are not in any compiler
>>> conditional.
>>>
>>> I spent about 2 hours looking into and fixing bug 250. I spent another
>>> couple of hours looking into a current, crash-causing bug that hasn't
>>> been given a bugzilla number. I haven't fixed it, as it's inconsistent
>>> as to how to cause it. Somehow, sometimes, a track name is getting set
>>> to a bad pointer, and it's related to changes you made for NoteTrack
>>> (using mTrack to mean a WaveTrack sometimes or a NoteTrack other times).
>>> Anyway, both these bugs caused crashes with compile flag settings that
>>> we intend to have for 2.0.
>>>
>>> Due to the fact that your changes in commit 10680 are, as you wrote,
>>> extensive, I'd like to back out all those changes and any other recent
>>> changes you've made for NoteTrack/MIDI. They have simply introduced too
>>> much instability and are not ready for prime time. I appreciate all your
>>> effort on this, but I think these should wait until post-2.0, as the
>>> intention was that they will not be activated until then anyway.
>>>
>>> In your commit 10680 remarks, some of the changes were to "improve
>>> NoteTrack display and (some) editing", so there may be a case for
>>> leaving some of those in. But definitely in MixerBoard.*, all the
>>> changes are for displaying NoteTracks in MixerBoard and should have been
>>> under EXPERIMENTAL_MIDI_OUT so they wouldn't affect code stability. I
>>> expect similar problems are in other parts of these changes. It might be
>>> possible to go through all that code and fix it to use
>>> EXPERIMENTAL_MIDI_OUT, but that's a lot of work, and is open to the
>>> possibility of mistakes/oversights that would cause instability.
>>> Overall, I'd rather just back out all these changes. We've had a very
>>> long beta cycle and I do not want to see that much uncertainty
>>> introduced at this point.
>>>
>>> As I commented on the velocity slider being a faked version of a gain
>>> slider, and on the changes to MixerBoard TrackClusters having special
>>> cases for NoteTrack, I think a lot of these changes would be better in a
>>> more OOP style. For example, your changes to TrackClusters have them
>>> switching between displaying NoteTracks and WaveTracks when they're
>>> moved around. Far better, I think, to have TrackCluster subclasses for
>>> each type of track, so they stay what they are, and just move around.
>>> That work is harder to do under compiler flags, so that's another reason
>>> to postpone this to post-2.0 -- i.e., do it the OOP way (the "right"
>>> way) all at once, when we can tolerate some instability, rather than
>>> compiler flags and conditionals on track type now, when instability is a
>>> huge problem.
>>>
>>> I'm willing to do the back-out if you don't have time. If you'd like to
>>> take a snapshot or create a branch of the current code to make it easier
>>> to get back to this work post-2.0, let me know. Otherwise, I'll do the
>>> back-out in a day or two, and you can retrieve the current code from
>>> commit 10680 (and 10755).
>>>
>>> Thanks,
>>> Vaughan
>>>
>>>
>
>