From 30a9c2d05b422dc60e9f7c3e1cb35cd9d02accf2 Mon Sep 17 00:00:00 2001
From: Colin Fletcher <colin.m.fletcher@googlemail.com>
Date: Mon, 1 Dec 2014 20:12:51 +0000
Subject: [PATCH] Comments in various call-sites of coverage()

Comments in various call sites of Evoral::coverage() marking things I think
are dubious (with XXX). Also straightened up the alignment of some ASCII
art in libs/ardour/diskstream.cc
---
 libs/ardour/audio_diskstream.cc |  2 ++
 libs/ardour/diskstream.cc       | 16 ++++++++--------
 libs/ardour/midi_diskstream.cc  |  1 +
 libs/ardour/playlist.cc         |  1 +
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/libs/ardour/audio_diskstream.cc b/libs/ardour/audio_diskstream.cc
index cf0e587ec6..fefc1c235c 100644
--- a/libs/ardour/audio_diskstream.cc
+++ b/libs/ardour/audio_diskstream.cc
@@ -460,6 +460,8 @@ AudioDiskstream::process (BufferSet& bufs, framepos_t transport_frame, pframes_t
 	if (record_enabled()) {
 
 		Evoral::OverlapType ot = Evoral::coverage (first_recordable_frame, last_recordable_frame, transport_frame, transport_frame + nframes);
+		// XXX should this be transport_frame + nframes - 1 ? coverage() expects its parameter ranges to include their end points
+		// XXX also, first_recordable_frame & last_recordable_frame may both be == max_framepos: coverage() will return OverlapNone in that case. Is thak OK?
 		calculate_record_range (ot, transport_frame, nframes, rec_nframes, rec_offset);
 
 		DEBUG_TRACE (DEBUG::CaptureAlignment, string_compose ("%1: this time record %2 of %3 frames, offset %4\n", _name, rec_nframes, nframes, rec_offset));
diff --git a/libs/ardour/diskstream.cc b/libs/ardour/diskstream.cc
index e24e678959..41f1eccdf7 100644
--- a/libs/ardour/diskstream.cc
+++ b/libs/ardour/diskstream.cc
@@ -700,16 +700,16 @@ Diskstream::calculate_record_range (Evoral::OverlapType ot, framepos_t transport
 
 	case Evoral::OverlapInternal:
 		/*     ----------    recrange
-		         |---|       transrange
-		*/
+		 *       |---|       transrange
+		 */
 		rec_nframes = nframes;
 		rec_offset = 0;
 		break;
 
 	case Evoral::OverlapStart:
 		/*    |--------|    recrange
-	        -----|          transrange
-		*/
+		 *  -----|          transrange
+		 */
 		rec_nframes = transport_frame + nframes - first_recordable_frame;
 		if (rec_nframes) {
 			rec_offset = first_recordable_frame - transport_frame;
@@ -718,16 +718,16 @@ Diskstream::calculate_record_range (Evoral::OverlapType ot, framepos_t transport
 
 	case Evoral::OverlapEnd:
 		/*    |--------|    recrange
-		         |--------  transrange
-		*/
+		 *       |--------  transrange
+		 */
 		rec_nframes = last_recordable_frame - transport_frame;
 		rec_offset = 0;
 		break;
 
 	case Evoral::OverlapExternal:
 		/*    |--------|    recrange
-		    --------------  transrange
-		*/
+		 *  --------------  transrange
+		 */
 		rec_nframes = last_recordable_frame - first_recordable_frame;
 		rec_offset = first_recordable_frame - transport_frame;
 		break;
diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc
index 9bc01b9bd4..27ec409b15 100644
--- a/libs/ardour/midi_diskstream.cc
+++ b/libs/ardour/midi_diskstream.cc
@@ -365,6 +365,7 @@ MidiDiskstream::process (BufferSet& bufs, framepos_t transport_frame, pframes_t
 
 	if (nominally_recording || (re && was_recording && _session.get_record_enabled() && _session.config.get_punch_in())) {
 		Evoral::OverlapType ot = Evoral::coverage (first_recordable_frame, last_recordable_frame, transport_frame, transport_frame + nframes);
+		// XXX should this be transport_frame + nframes - 1 ? coverage() expects its parameter ranges to include their end points
 
 		calculate_record_range(ot, transport_frame, nframes, rec_nframes, rec_offset);
 		/* For audio: not writing frames to the capture ringbuffer offsets
diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc
index ff2f524398..7a990809a1 100644
--- a/libs/ardour/playlist.cc
+++ b/libs/ardour/playlist.cc
@@ -3222,6 +3222,7 @@ restart:
 				continue;
 			}
 
+			// XXX i->from can be > i->to - is this right? coverage() will return OverlapNone in this case
 			if (Evoral::coverage (i->from, i->to, j->from, j->to) != Evoral::OverlapNone) {
 				i->from = min (i->from, j->from);
 				i->to = max (i->to, j->to);