[Audacity-devel] I merged more of Pokechu22's changes

[Paul Licameli]

I am satisfied with the changes in MixerBoard. It is pleasing that
UpdateTrackClusters now needs no changes.

I merged one of the commits that affect TrackPanel.cpp, but with some
changes. Another thing I mean to do is make Track::GetKind a private
function. So I rewrote a bit to lessen the number of new calls to it.

It is good to see the drawing hacks go away and to make the drawing of wave
and note tracks more analogous.

It remains for me to understand EXPERIMENTAL_MIDI_CONTROLS and related
changes to TrackPanel.cpp.

PRL

[Paul Licameli]

Poke, trying to understand EXPERIMENTAL_MIDI_CONTROLS:


1. I think there should be no new EXPERIMENTAL flag. This is (almost)
an enhancement only to the visualization of MIDI data, independent of
whether we can play it. And too many #ifdefs are cluttersome. So I say,
just let USE_MIDI control this too; and USE_MIDI is 1 by default, so this
will make an enhancement to Audacity 2.2.0 even if MIDI playback isn't
ready.
2. I just tried it for myself and saw that yes, there are invisible,
clickable buttons in master, for showing and hiding MIDI channels!
Consider it a fixed bug that we will always see the buttons on note tracks
instead, if track height is great enough.
3. But I still find that there are invisible but clickable buttons, if I
shorten the note track just enough so that the buttons disappear. Fix that.
4. When channels change visibility, MakeParentModifyState should be
called, so that if you then make an edit and undo, the visibility state is
restored as it was before edit. This is what we do with other view changes
too like resizing of tracks which don't have their own undo items.
5. I said "almost" above. The exception is
NoteTrack::WarpAndTransposeNotes. (This function is not reachable unless
you un-define USE_SBSMS, and then use the Change Pitch or Change Tempo
effect.)
1. Transpositions were conditional on channel visibility, but warps
were not! I question the correctness of the old though unreachable code.
2. You change things to make transposition dependent on visibility or
not, depending on your conditional compilation flag.
3. I think the right thing is to make it all unconditional on
visibility. What an effect does to WaveTracks is independent of whether
they are muted, and I think the right thing is to do analogously for MIDI.
4. This bug fix merits a separate commit.

That's all, about that commit.

PRL

Post by Paul Licameli
I am satisfied with the changes in MixerBoard. It is pleasing that
UpdateTrackClusters now needs no changes.
I merged one of the commits that affect TrackPanel.cpp, but with some
changes. Another thing I mean to do is make Track::GetKind a private
function. So I rewrote a bit to lessen the number of new calls to it.
It is good to see the drawing hacks go away and to make the drawing of
wave and note tracks more analogous.
It remains for me to understand EXPERIMENTAL_MIDI_CONTROLS and related
changes to TrackPanel.cpp.
PRL

[Paul Licameli]

Poke,

Reviewing "Use mTimeTrack timing rather than mMidiPlaySpeed"

I did not try to understand every detail, but I observe that looping play
isn't right.

Is the paragraph about "Midi Time" in comments at top of AudioIO.cpp all
correct?

About your comment in MidiThread::Entry -- I know it is no longer true that
TrackPanel::OnTimer fires every 200 ms. It is now 50. That number is
kTimerInterval in TrackPanel.h.

About the four uses of ComputeWarpedLength that you added -- I think you
are not consistently using it as intended. It computes a "warped" (real,
playback-time) duration from two track times. It integrates the reciprocal
of "speed" -- speed being track time divided by real time.

But in the first use of ComputeWarpedLength, the original code multiplied
by speed, rather than dividing. So I think you need to solve the inverse
problem. I think the variable realTime oddly stores a real time duration
added to mT0 (a track time point). TimeTrack::SolveWarpedLength is what
you need.

In the second use do you want to find mT0 plus warped time? You do not add
mT0.

In the third place, again the original code multiplied by speed, not
dividing, so again it is the inverse to computing warped length.

In the fourth place, I think it is correct.

But maybe I'm just all confused by this confusing confusion too.

PRL

Post by Paul Licameli
1. I think there should be no new EXPERIMENTAL flag. This is (almost)
an enhancement only to the visualization of MIDI data, independent of
whether we can play it. And too many #ifdefs are cluttersome. So I say,
just let USE_MIDI control this too; and USE_MIDI is 1 by default, so this
will make an enhancement to Audacity 2.2.0 even if MIDI playback isn't
ready.
2. I just tried it for myself and saw that yes, there are invisible,
clickable buttons in master, for showing and hiding MIDI channels!
Consider it a fixed bug that we will always see the buttons on note tracks
instead, if track height is great enough.
3. But I still find that there are invisible but clickable buttons, if
I shorten the note track just enough so that the buttons disappear. Fix
that.
4. When channels change visibility, MakeParentModifyState should be
called, so that if you then make an edit and undo, the visibility state is
restored as it was before edit. This is what we do with other view changes
too like resizing of tracks which don't have their own undo items.
5. I said "almost" above. The exception is NoteTrack::WarpAndTransposeNotes.
(This function is not reachable unless you un-define USE_SBSMS, and then
use the Change Pitch or Change Tempo effect.)
1. Transpositions were conditional on channel visibility, but warps
were not! I question the correctness of the old though unreachable code.
2. You change things to make transposition dependent on visibility
or not, depending on your conditional compilation flag.
3. I think the right thing is to make it all unconditional on
visibility. What an effect does to WaveTracks is independent of whether
they are muted, and I think the right thing is to do analogously for MIDI.
4. This bug fix merits a separate commit.
That's all, about that commit.
PRL

Post by Paul Licameli
I am satisfied with the changes in MixerBoard. It is pleasing that
UpdateTrackClusters now needs no changes.
I merged one of the commits that affect TrackPanel.cpp, but with some
changes. Another thing I mean to do is make Track::GetKind a private
function. So I rewrote a bit to lessen the number of new calls to it.
It is good to see the drawing hacks go away and to make the drawing of
wave and note tracks more analogous.
It remains for me to understand EXPERIMENTAL_MIDI_CONTROLS and related
changes to TrackPanel.cpp.
PRL