Quantcast

I merged more of Pokechu22's changes

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: I merged more of Pokechu22's changes

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


On Sat, Apr 1, 2017 at 12:57 PM, Paul Licameli <[hidden email]> wrote:
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



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: I merged more of Pokechu22's changes

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



On Sat, Apr 1, 2017 at 2:28 PM, Paul Licameli <[hidden email]> wrote:
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


On Sat, Apr 1, 2017 at 12:57 PM, Paul Licameli <[hidden email]> wrote:
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




------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Loading...