From 2de98415bce64615b3c6ff543fb4d12318aefd66 Mon Sep 17 00:00:00 2001
From: Nicolas Pope <nicolas.pope@utu.fi>
Date: Thu, 23 Jan 2020 21:55:37 +0200
Subject: [PATCH] Resolves #288 frame buffer swap bug

---
 applications/gui/src/camera.cpp               |  4 ++++
 applications/gui/src/main.cpp                 | 10 ++++++++--
 applications/gui/src/src_window.cpp           | 14 +++++++-------
 applications/reconstruct/src/main.cpp         |  4 ++--
 components/audio/include/ftl/audio/frame.hpp  |  2 ++
 components/audio/src/source.cpp               |  8 +++++---
 components/control/cpp/src/master.cpp         |  3 ++-
 .../rgbd-sources/include/ftl/rgbd/frame.hpp   |  4 ++++
 components/rgbd-sources/src/frame.cpp         |  4 ++--
 components/rgbd-sources/src/frameset.cpp      | 10 +++++++---
 components/streams/src/receiver.cpp           | 19 ++++++++++++-------
 .../structures/include/ftl/data/frame.hpp     |  5 ++++-
 12 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/applications/gui/src/camera.cpp b/applications/gui/src/camera.cpp
index 9aba01850..8f2162dc8 100644
--- a/applications/gui/src/camera.cpp
+++ b/applications/gui/src/camera.cpp
@@ -227,6 +227,10 @@ void ftl::gui::Camera::_draw(ftl::rgbd::FrameSet &fs) {
 	Eigen::Matrix4d t;
 	t.setIdentity();
 	renderer_->render(fs, frame_, channel_, t);
+
+	// TODO: Insert post-render pipeline.
+	// FXAA + Bad colour removal
+
 	_downloadFrames(&frame_);
 
 	if (record_stream_ && record_stream_->active()) {
diff --git a/applications/gui/src/main.cpp b/applications/gui/src/main.cpp
index cd52e88ba..6b1867b06 100644
--- a/applications/gui/src/main.cpp
+++ b/applications/gui/src/main.cpp
@@ -36,11 +36,16 @@ int main(int argc, char **argv) {
 	try {
 		nanogui::init();
 
-		/* scoped variables */ {
+		{
 			nanogui::ref<ftl::gui::Screen> app = new ftl::gui::Screen(root, net, controller);
 			app->drawAll();
 			app->setVisible(true);
 			nanogui::mainloop();
+
+			LOG(INFO) << "Stopping...";
+			ftl::timer::stop(false);
+			ftl::pool.stop(true);
+			LOG(INFO) << "All threads stopped.";
 		}
 
 		nanogui::shutdown();
@@ -54,7 +59,8 @@ int main(int argc, char **argv) {
 		return -1;
 	}
 
-	net->shutdown();
+
+	net->shutdown();	
 	delete controller;
 	delete net;
 	delete root;
diff --git a/applications/gui/src/src_window.cpp b/applications/gui/src/src_window.cpp
index 01eaf3a17..9681239c3 100644
--- a/applications/gui/src/src_window.cpp
+++ b/applications/gui/src/src_window.cpp
@@ -80,6 +80,13 @@ SourceWindow::SourceWindow(ftl::gui::Screen *screen)
 
 	cycle_ = 0;
 	receiver_->onFrameSet([this](ftl::rgbd::FrameSet &fs) {
+		// Enforce interpolated colour
+		for (int i=0; i<fs.frames.size(); ++i) {
+			fs.frames[i].createTexture<uchar4>(Channel::Colour, true);
+		}
+
+		pre_pipeline_->apply(fs, fs, 0);
+
 		fs.swapTo(frameset_);
 
 		// Request the channels required by current camera configuration
@@ -96,13 +103,6 @@ SourceWindow::SourceWindow(ftl::gui::Screen *screen)
 
 		//LOG(INFO) << "Channels = " << (unsigned int)cstream->available(fs.id);
 
-		// Enforce interpolated colour
-		for (int i=0; i<frameset_.frames.size(); ++i) {
-			frameset_.frames[i].createTexture<uchar4>(Channel::Colour, true);
-		}
-
-		pre_pipeline_->apply(frameset_, frameset_, 0);
-
 		int i=0;
 		for (auto cam : cameras_) {
 			// Only update the camera periodically unless the active camera
diff --git a/applications/reconstruct/src/main.cpp b/applications/reconstruct/src/main.cpp
index 9869a7915..afc9f92e2 100644
--- a/applications/reconstruct/src/main.cpp
+++ b/applications/reconstruct/src/main.cpp
@@ -266,11 +266,11 @@ static void run(ftl::Configurable *root) {
 
 	LOG(INFO) << "Shutting down...";
 	ftl::timer::stop();
+	ftl::pool.stop(true);
 	ctrl.stop();
 	net->shutdown();
-	ftl::pool.stop();
 
-	cudaProfilerStop();
+	//cudaProfilerStop();
 
 	LOG(INFO) << "Deleting...";
 
diff --git a/components/audio/include/ftl/audio/frame.hpp b/components/audio/include/ftl/audio/frame.hpp
index 854698609..efd333849 100644
--- a/components/audio/include/ftl/audio/frame.hpp
+++ b/components/audio/include/ftl/audio/frame.hpp
@@ -30,6 +30,8 @@ struct AudioData {
 		throw ftl::exception("Type not valid for audio channel");
 	}
 
+	inline void reset() {}
+
 	Audio data;
 };
 
diff --git a/components/audio/src/source.cpp b/components/audio/src/source.cpp
index 904433edf..97aeac77d 100644
--- a/components/audio/src/source.cpp
+++ b/components/audio/src/source.cpp
@@ -47,13 +47,15 @@ Source::Source(nlohmann::json &config) : ftl::Configurable(config), buffer_(4800
 	#ifdef HAVE_PORTAUDIO
     ftl::audio::pa_init();
 
+	int device = value("audio_device",-1);
+	if (device >= Pa_GetDeviceCount()) device = -1;
+
     PaStreamParameters inputParameters;
     //bzero( &inputParameters, sizeof( inputParameters ) );
     inputParameters.channelCount = 2;
-    inputParameters.device = value("audio_device",-1);
-    //inputParameters.hostApiSpecificStreamInfo = NULL;
+    inputParameters.device = device;
     inputParameters.sampleFormat = paInt16;
-    inputParameters.suggestedLatency = (inputParameters.device >= 0) ? Pa_GetDeviceInfo(inputParameters.device)->defaultLowInputLatency : 0;
+    inputParameters.suggestedLatency = (device >= 0) ? Pa_GetDeviceInfo(device)->defaultLowInputLatency : 0;
     inputParameters.hostApiSpecificStreamInfo = NULL;
 
 	PaError err;
diff --git a/components/control/cpp/src/master.cpp b/components/control/cpp/src/master.cpp
index 059dfe914..4c1354bee 100644
--- a/components/control/cpp/src/master.cpp
+++ b/components/control/cpp/src/master.cpp
@@ -99,7 +99,7 @@ Master::Master(Configurable *root, Universe *net)
 }
 
 Master::~Master() {
-	net_->unbind("log");
+	stop();
 }
 
 void Master::restart() {
@@ -214,6 +214,7 @@ void Master::stop() {
 	net_->unbind("get_cfg");
 	net_->unbind("slave_details"); // TODO: Remove
 	net_->unbind("log_subscribe");
+	net_->unbind("log");
 }
 
 void Master::sendLog(const loguru::Message& message) {
diff --git a/components/rgbd-sources/include/ftl/rgbd/frame.hpp b/components/rgbd-sources/include/ftl/rgbd/frame.hpp
index b894cf07c..65837020f 100644
--- a/components/rgbd-sources/include/ftl/rgbd/frame.hpp
+++ b/components/rgbd-sources/include/ftl/rgbd/frame.hpp
@@ -51,6 +51,10 @@ struct VideoData {
 	T &make() {
 		throw ftl::exception("Unsupported type for Video data channel");
 	};
+
+	inline void reset() {
+		encoded.clear();
+	}
 };
 
 // Specialisations for cv mat types
diff --git a/components/rgbd-sources/src/frame.cpp b/components/rgbd-sources/src/frame.cpp
index 37ceb7b28..2a9765cee 100644
--- a/components/rgbd-sources/src/frame.cpp
+++ b/components/rgbd-sources/src/frame.cpp
@@ -56,9 +56,9 @@ cv::cuda::GpuMat &VideoData::make<cv::cuda::GpuMat>() {
 	for (size_t i=0u; i<Channels<0>::kMax; ++i) {
 		data_[i].encoded.clear();
 	}
-}
+}*/
 
-void Frame::resetFull() {
+/*void Frame::resetFull() {
 	origin_ = nullptr;
 	channels_.clear();
 	gpu_.clear();
diff --git a/components/rgbd-sources/src/frameset.cpp b/components/rgbd-sources/src/frameset.cpp
index 4c9a95eb5..8064aaff0 100644
--- a/components/rgbd-sources/src/frameset.cpp
+++ b/components/rgbd-sources/src/frameset.cpp
@@ -37,6 +37,7 @@ void FrameSet::swapTo(ftl::rgbd::FrameSet &fs) {
 	fs.count = static_cast<int>(count);
 	fs.stale = stale;
 	fs.mask = static_cast<unsigned int>(mask);
+	fs.id = id;
 
 	for (size_t i=0; i<frames.size(); ++i) {
 		frames[i].swapTo(Channels<0>::All(), fs.frames[i]);
@@ -225,7 +226,7 @@ ftl::rgbd::FrameSet *Builder::_findFrameset(int64_t ts) {
  * Note: Must occur inside a mutex lock.
  */
 ftl::rgbd::FrameSet *Builder::_getFrameset() {
-	LOG(INFO) << "BUF SIZE = " << framesets_.size();
+	//LOG(INFO) << "BUF SIZE = " << framesets_.size();
 	for (auto i=framesets_.begin(); i!=framesets_.end(); i++) {
 		auto *f = *i;
 		//LOG(INFO) << "GET: " << f->count << " of " << size_;
@@ -266,8 +267,11 @@ ftl::rgbd::FrameSet *Builder::_addFrameset(int64_t timestamp) {
 		if (framesets_.size() < kMaxFramesets) {
 			allocated_.push_back(new ftl::rgbd::FrameSet);
 		} else {
-			LOG(ERROR) << "Could not allocate framesetL: " << timestamp;
-			return nullptr;
+			LOG(WARNING) << "Frameset buffer full, resetting: " << timestamp;
+
+			// Do a complete reset
+			std::swap(framesets_, allocated_);
+			//return nullptr;
 		}
 	}
 	FrameSet *newf = allocated_.front();
diff --git a/components/streams/src/receiver.cpp b/components/streams/src/receiver.cpp
index 8ed25bf4c..cba34010b 100644
--- a/components/streams/src/receiver.cpp
+++ b/components/streams/src/receiver.cpp
@@ -22,7 +22,10 @@ Receiver::Receiver(nlohmann::json &config) : ftl::Configurable(config), stream_(
 }
 
 Receiver::~Receiver() {
-
+	//if (stream_) {
+	//	stream_->onPacket(nullptr);
+	//}
+	builder_.onFrameSet(nullptr);
 }
 
 void Receiver::onAudio(const ftl::audio::FrameSet::Callback &cb) {
@@ -151,13 +154,15 @@ void Receiver::_processVideo(const StreamPacket &spkt, const Packet &pkt) {
 		//if (rchan != Channel::Colour && rchan != chan) return;
 
 		if (frame.frame.hasChannel(spkt.channel)) {
-			// FIXME: Is this a corruption in recording or in playback?
-			// Seems to occur in same place in ftl file, one channel is missing
-			LOG(ERROR) << "Previous frame not complete: " << frame.timestamp;
-			//LOG(ERROR) << " --- " << (string)spkt;
 			UNIQUE_LOCK(frame.mutex, lk);
-			frame.frame.reset();
-			frame.completed.clear();
+			//if (frame.frame.hasChannel(spkt.channel)) {
+				// FIXME: Is this a corruption in recording or in playback?
+				// Seems to occur in same place in ftl file, one channel is missing
+				LOG(WARNING) << "Previous frame not complete: " << frame.timestamp;
+				//LOG(ERROR) << " --- " << (string)spkt;
+				//frame.frame.reset();
+				//frame.completed.clear();
+			//}
 		}
 		frame.timestamp = spkt.timestamp;
 
diff --git a/components/structures/include/ftl/data/frame.hpp b/components/structures/include/ftl/data/frame.hpp
index eb8ae3e00..5d717f6c4 100644
--- a/components/structures/include/ftl/data/frame.hpp
+++ b/components/structures/include/ftl/data/frame.hpp
@@ -270,6 +270,9 @@ void ftl::data::Frame<BASE,N,STATE,DATA>::reset() {
 	origin_ = nullptr;
 	channels_.clear();
 	data_channels_.clear();
+	for (size_t i=0u; i<ftl::codecs::Channels<BASE>::kMax; ++i) {
+		data_[i].reset();
+	}
 }
 
 template <int BASE, int N, typename STATE, typename DATA>
@@ -283,7 +286,7 @@ void ftl::data::Frame<BASE,N,STATE,DATA>::swapTo(ftl::codecs::Channels<BASE> cha
 		// Should we swap this channel?
 		if (channels.has(c)) {
 			f.channels_ += c;
-			f.getData(c) = std::move(getData(c));
+			std::swap(f.getData(c),getData(c));
 		}
 	}
 
-- 
GitLab