[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Cleanup and generalization: get rid of Audio_column. (issue 42600043
From: |
dak |
Subject: |
Re: Cleanup and generalization: get rid of Audio_column. (issue 42600043) |
Date: |
Fri, 20 Dec 2013 16:46:19 +0000 |
https://codereview.appspot.com/42600043/diff/20001/lily/audio-item.cc
File lily/audio-item.cc (right):
https://codereview.appspot.com/42600043/diff/20001/lily/audio-item.cc#newcode44
lily/audio-item.cc:44: return int (moment_to_ticks (when_));
Why cast to int here? moment_to_ticks is already an int.
https://codereview.appspot.com/42600043/diff/20001/lily/audio-item.cc#newcode201
lily/audio-item.cc:201: Audio_tempo::Audio_tempo (Moment when, int
per_minute_4)
per_minute_4 warrants a comment. Yes, it was already missing one before
you started.
https://codereview.appspot.com/42600043/diff/20001/lily/control-track-performer.cc
File lily/control-track-performer.cc (right):
https://codereview.appspot.com/42600043/diff/20001/lily/control-track-performer.cc#newcode52
lily/control-track-performer.cc:52:
Control_track_performer::start_translation_timestep ()
Ok, here is a filthy bad thing: in issue 2392 I finally punted and gave
up on start_translation_timestep in implicitly created contexts to be
reliably run.
That means that for implicitly created contexts from a
Simple_music_iterator, start_translation_timestep will only get run at
the timestep _after_ creation.
If you think you can fix _that_, go ahead (you are obviously more into
the Midi code than I was, but that's more a problem of the iterator
framework).
If not, this code will likely not do what you think it should do in all
circumstances.
https://codereview.appspot.com/42600043/diff/20001/lily/include/lily-proto.hh
File lily/include/lily-proto.hh (left):
https://codereview.appspot.com/42600043/diff/20001/lily/include/lily-proto.hh#oldcode26
lily/include/lily-proto.hh:26: class Audio_column;
Good call: this is easily forgotten, and I recently cleaned out a few
long gone classes from here.
https://codereview.appspot.com/42600043/