Align punch in/out recording with latency-compensation

libs/ardour/disk_writer.cc

@@ -89,18 +89,14 @@ void
 DiskWriter::check_record_status (samplepos_t transport_sample, double speed, bool can_record)
 {
 	int possibly_recording;
-	int rolling;
-	int change;
 	const int transport_rolling = 0x4;
 	const int track_rec_enabled = 0x2;
 	const int global_rec_enabled = 0x1;
-	const int fully_rec_enabled = (transport_rolling|track_rec_enabled|global_rec_enabled);
+	const int fully_rec_enabled = (transport_rolling |track_rec_enabled | global_rec_enabled);
 
 	/* merge together the 3 factors that affect record status, and compute what has changed. */
 
-	rolling = speed != 0.0f;
-	possibly_recording = (rolling << 2) | ((int)record_enabled() << 1) | (int)can_record;
-	change = possibly_recording ^ last_possibly_recording;
+	possibly_recording = (speed != 0.0f ? 4 : 0)  | (record_enabled() ? 2 : 0) | (can_record ? 1 : 0);
 
 	if (possibly_recording == last_possibly_recording) {
 		return;
@@ -112,14 +108,32 @@ DiskWriter::check_record_status (samplepos_t transport_sample, double speed, boo
 			return;
 		}
 
-		capture_start_sample = _session.transport_sample ();
+		Location* loc;
+		if  (_session.config.get_punch_in () && 0 != (loc = _session.locations()->auto_punch_location ())) {
+			capture_start_sample = loc->start ();
+		} else {
+			capture_start_sample = _session.transport_sample ();
+		}
+
 		first_recordable_sample = capture_start_sample;
 
 		if (_alignment_style == ExistingMaterial) {
 			first_recordable_sample += _capture_offset + _playback_offset;
 		}
 
-		last_recordable_sample = max_samplepos;
+		if  (_session.config.get_punch_out () && 0 != (loc = _session.locations()->auto_punch_location ())) {
+			/* this freezes the punch-out point when starting to record.
+			 *
+			 * We should allow to move it or at least allow to disable punch-out
+			 * while rolling..
+			 */
+			last_recordable_sample = loc->end ();
+			if (_alignment_style == ExistingMaterial) {
+				last_recordable_sample += _capture_offset + _playback_offset;
+			}
+		} else {
+			last_recordable_sample = max_samplepos;
+		}
 
 		DEBUG_TRACE (DEBUG::CaptureAlignment, string_compose ("%1: @ %2 (STS: %3) CS:%4 FRS: %5 IL: %7, OL: %8 CO: %r9 PO: %10 WOL: %11 WIL: %12\n",
 		                                                      name(),
@@ -138,29 +152,6 @@ DiskWriter::check_record_status (samplepos_t transport_sample, double speed, boo
 
 		prepare_record_status (capture_start_sample);
 
-	} else {
-
-		if (last_possibly_recording == fully_rec_enabled) {
-
-			/* we were recording last time */
-
-			if (change & transport_rolling) {
-
-				/* transport-change (stopped rolling): last_recordable_sample was set in ::prepare_to_stop(). We
-				 * had to set it there because we likely rolled past the stopping point to declick out,
-				 * and then backed up.
-				 */
-
-			} else {
-				/* punch out */
-
-				last_recordable_sample = _session.transport_sample();
-
-				if (_alignment_style == ExistingMaterial) {
-					//XXX
-				}
-			}
-		}
 	}
 
 	last_possibly_recording = possibly_recording;
@@ -366,7 +357,9 @@ DiskWriter::run (BufferSet& bufs, samplepos_t start_sample, samplepos_t end_samp
 	bool nominally_recording;
 
 	bool re = record_enabled ();
+	bool punch_in = _session.config.get_punch_in () && _session.locations()->auto_punch_location ();
 	bool can_record = _session.actively_recording ();
+	can_record |= _session.get_record_enabled () && punch_in && _session.transport_sample () <= _session.locations()->auto_punch_location ()->start ();
 
 	_need_butler = false;
 
@@ -400,7 +393,7 @@ DiskWriter::run (BufferSet& bufs, samplepos_t start_sample, samplepos_t end_samp
 		get_location_times (loop_loc, &loop_start, &loop_end, &loop_length);
 	}
 
-	if (nominally_recording || (re && was_recording && _session.get_record_enabled() && _session.config.get_punch_in ())) {
+	if (nominally_recording || (re && was_recording && _session.get_record_enabled() && punch_in)) {
 
 		Evoral::OverlapType ot = Evoral::coverage (first_recordable_sample, last_recordable_sample, start_sample, end_sample);
 		// XXX should this be transport_sample + nframes - 1 ? coverage() expects its parameter ranges to include their end points

libs/ardour/route.cc

@@ -362,6 +362,8 @@ Route::process_output_buffers (BufferSet& bufs,
 	start_sample += _output->latency ();
 	end_sample += _output->latency ();
 
+	const double speed = (is_auditioner() ? 1.0 : _session.transport_speed ());
+
 	/* Note: during intial pre-roll 'start_sample' as passed as argument can be negative.
 	 * Functions calling process_output_buffers() will set  "run_disk_reader"
 	 * to false if the pre-roll count-down is larger than playback_latency ().
@@ -380,9 +382,10 @@ Route::process_output_buffers (BufferSet& bufs,
 	 * given that
 	 */
 	bool run_disk_writer = false;
-	if (_disk_writer) {
-		samplecnt_t latency_preroll = _session.remaining_latency_preroll ();
-		run_disk_writer = latency_preroll < nframes + (_signal_latency + _output->latency ());
+	if (_disk_writer && speed != 0) {
+		if (end_sample - _disk_writer->input_latency () < _session.transport_sample ()) {
+			run_disk_writer = true;
+		}
 	}
 
 	/* Tell main outs what to do about monitoring.  We do this so that
@@ -391,6 +394,12 @@ Route::process_output_buffers (BufferSet& bufs,
 	 * is true.
 	 *
 	 * We override this in the case where we have an internal generator.
+	 *
+	 * FIXME: when punching in/out this also depends on latency compensated time
+	 * for this route. monitoring_state() does not currently handle that correctly,.
+	 *
+	 * Also during remaining_latency_preroll, transport_rolling () is false, but
+	 * we may need to monitor disk instead.
 	 */
 	bool silence = _have_internal_generator ? false : (monitoring_state () == MonitoringSilence);
 
@@ -473,7 +482,6 @@ Route::process_output_buffers (BufferSet& bufs,
 	bool const meter_already_run = metering_state() == MeteringInput;
 
 	samplecnt_t latency = 0;
-	const double speed = (is_auditioner() ? 1.0 : _session.transport_speed ());
 
 	for (ProcessorList::const_iterator i = _processors.begin(); i != _processors.end(); ++i) {
 
@@ -4068,12 +4076,12 @@ Route::update_signal_latency (bool apply_to_delayline)
 	_signal_latency = l_out;
 
 	for (ProcessorList::iterator i = _processors.begin(); i != _processors.end(); ++i) {
-		if ((*i)->active ()) {
-			l_in += (*i)->signal_latency ();
-		}
 		(*i)->set_input_latency (l_in);
 		(*i)->set_playback_offset (_signal_latency + _output->latency ());
 		(*i)->set_capture_offset (_input->latency ());
+		if ((*i)->active ()) {
+			l_in += (*i)->signal_latency ();
+		}
 	}
 
 	lm.release ();
@@ -5974,7 +5982,17 @@ Route::monitoring_state () const
 	 * sept 26th 2012, we differentiate between the cases where punch is
 	 * enabled and those where it is not.
 	 *
-	 * rg: I suspect this is not the case: monitoring may differ
+	 * rg: sept 30 2017: Above is not the case: punch-in/out location is
+	 * global session playhead position.
+	 * When this method is called from process_output_buffers() we need
+	 * to use delay-compensated route's process-position.
+	 *
+	 * NB. Disk reader/writer may also be offset by a same amount of time.
+	 *
+	 * Also keep in mind that _session.transport_rolling() is false during
+	 * pre-roll but the disk already produces output.
+	 *
+	 * TODO: FIXME
 	 */
 
 	if (_session.config.get_punch_in() || _session.config.get_punch_out()) {

libs/ardour/session.cc

@@ -1655,7 +1655,7 @@ Session::auto_punch_start_changed (Location* location)
 {
 	replace_event (SessionEvent::PunchIn, location->start());
 
-	if (get_record_enabled() && config.get_punch_in()) {
+	if (get_record_enabled() && config.get_punch_in() && !actively_recording ()) {
 		/* capture start has been changed, so save new pending state */
 		save_state ("", true);
 	}
@@ -1664,19 +1664,14 @@ Session::auto_punch_start_changed (Location* location)
 void
 Session::auto_punch_end_changed (Location* location)
 {
-	samplepos_t when_to_stop = location->end();
-	// when_to_stop += _worst_output_latency + _worst_input_latency;
-	replace_event (SessionEvent::PunchOut, when_to_stop);
+	replace_event (SessionEvent::PunchOut, location->end());
 }
 
 void
 Session::auto_punch_changed (Location* location)
 {
-	samplepos_t when_to_stop = location->end();
-
-	replace_event (SessionEvent::PunchIn, location->start());
-	//when_to_stop += _worst_output_latency + _worst_input_latency;
-	replace_event (SessionEvent::PunchOut, when_to_stop);
+	auto_punch_start_changed (location);
+	auto_punch_end_changed (location);
 }
 
 /** @param loc A loop location.
@@ -1754,7 +1749,7 @@ Session::set_auto_punch_location (Location* location)
 	if ((existing = _locations->auto_punch_location()) != 0 && existing != location) {
 		punch_connections.drop_connections();
 		existing->set_auto_punch (false, this);
-		remove_event (existing->start(), SessionEvent::PunchIn);
+		clear_events (SessionEvent::PunchIn);
 		clear_events (SessionEvent::PunchOut);
 		auto_punch_location_changed (0);
 	}

libs/ardour/session_state.cc

@@ -4038,9 +4038,9 @@ Session::config_changed (std::string p, bool ours)
 		if ((location = _locations->auto_punch_location()) != 0) {
 
 			if (config.get_punch_in ()) {
-				replace_event (SessionEvent::PunchIn, location->start());
+				auto_punch_start_changed (location);
 			} else {
-				remove_event (location->start(), SessionEvent::PunchIn);
+				clear_events (SessionEvent::PunchIn);
 			}
 		}
 
@@ -4051,7 +4051,7 @@ Session::config_changed (std::string p, bool ours)
 		if ((location = _locations->auto_punch_location()) != 0) {
 
 			if (config.get_punch_out()) {
-				replace_event (SessionEvent::PunchOut, location->end());
+				auto_punch_end_changed (location);
 			} else {
 				clear_events (SessionEvent::PunchOut);
 			}

libs/ardour/session_transport.cc

@@ -1649,6 +1649,13 @@ Session::start_transport ()
 	switch (record_status()) {
 	case Enabled:
 		if (!config.get_punch_in()) {
+			/* This is only for UIs (keep blinking rec-en before
+			 * punch-in, don't show rec-region etc). The UI still
+			 * depends on SessionEvent::PunchIn and ensuing signals.
+			 *
+			 * The disk-writers handle punch in/out internally
+			 * in their local delay-compensated timeframe.
+			 */
 			enable_record ();
 		}
 		break;