Merge pull request #48 from DanielSWolf/bugfix/#46-progress

Fix progress reporting

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 * **Added** basic support for non-English recordings through phonetic recognition ([issue #45](https://github.com/DanielSWolf/rhubarb-lip-sync/issues/45)).
+* **Fixed** a bug that prevented the progress bar from reaching 100% ([issue #48](https://github.com/DanielSWolf/rhubarb-lip-sync/issues/48)).
 
 ## Version 1.8.0
 

rhubarb/CMakeLists.txt

@@ -467,6 +467,8 @@ add_library(rhubarb-tools
 	src/tools/parallel.h
 	src/tools/platformTools.cpp
 	src/tools/platformTools.h
+	src/tools/progress.cpp
+	src/tools/progress.h
 	src/tools/ProgressBar.cpp
 	src/tools/ProgressBar.h
 	src/tools/stringTools.cpp

rhubarb/src/audio/processing.cpp

@@ -1,4 +1,5 @@
 #include "processing.h"
+#include <algorithm>
 
 using std::function;
 using std::vector;

rhubarb/src/audio/processing.h

@@ -3,7 +3,7 @@
 #include <vector>
 #include <functional>
 #include "AudioClip.h"
-#include "tools/ProgressBar.h"
+#include "tools/progress.h"
 
 void process16bitAudioClip(const AudioClip& audioClip, std::function<void(const std::vector<int16_t>&)> processBuffer, size_t bufferCapacity, ProgressSink& progressSink);
 void process16bitAudioClip(const AudioClip& audioClip, std::function<void(const std::vector<int16_t>&)> processBuffer, ProgressSink& progressSink);

rhubarb/src/audio/voiceActivityDetection.h

@@ -1,6 +1,6 @@
 #pragma once
 #include "AudioClip.h"
 #include "time/BoundedTimeline.h"
-#include "tools/ProgressBar.h"
+#include "tools/progress.h"
 
 JoiningBoundedTimeline<void> detectVoiceActivity(const AudioClip& audioClip, int maxThreadCount, ProgressSink& progressSink);

rhubarb/src/lib/rhubarbLib.h

@@ -3,7 +3,7 @@
 #include "core/Shape.h"
 #include "time/ContinuousTimeline.h"
 #include "audio/AudioClip.h"
-#include "tools/ProgressBar.h"
+#include "tools/progress.h"
 #include <boost/filesystem.hpp>
 #include "animation/targetShapeSet.h"
 #include "recognition/Recognizer.h"

rhubarb/src/recognition/Recognizer.h

@@ -2,7 +2,7 @@
 
 #include "audio/AudioClip.h"
 #include "core/Phone.h"
-#include "tools/ProgressBar.h"
+#include "tools/progress.h"
 #include "time/BoundedTimeline.h"
 
 class Recognizer {

rhubarb/src/recognition/pocketSphinxTools.h

@@ -3,7 +3,7 @@
 #include "time/BoundedTimeline.h"
 #include "core/Phone.h"
 #include "audio/AudioClip.h"
-#include "tools/ProgressBar.h"
+#include "tools/progress.h"
 #include <boost/filesystem/path.hpp>
 
 extern "C" {

rhubarb/src/rhubarb/sinks.cpp

@@ -16,6 +16,7 @@ using boost::optional;
 
 NiceStderrSink::NiceStderrSink(Level minLevel) :
 	minLevel(minLevel),
+	progress(0.0),
 	innerSink(make_shared<StdErrSink>(make_shared<SimpleConsoleFormatter>()))
 {}
 
@@ -26,7 +27,8 @@ void NiceStderrSink::receive(const logging::Entry& entry) {
 		startProgressIndication();
 	} else if (const ProgressEntry* progressEntry = dynamic_cast<const ProgressEntry*>(&entry)) {
 		assert(progressBar);
-		progressBar->reportProgress(progressEntry->getProgress());
+		progress = progressEntry->getProgress();
+		progressBar->reportProgress(progress);
 	} else if (dynamic_cast<const SuccessEntry*>(&entry)) {
 		interruptProgressIndication();
 		std::cerr << "Done." << std::endl;
@@ -44,7 +46,6 @@ void NiceStderrSink::receive(const logging::Entry& entry) {
 void NiceStderrSink::startProgressIndication() {
 	std::cerr << "Progress: ";
 	progressBar = boost::in_place();
-	progressBar->setClearOnDestruction(false);
 }
 
 void NiceStderrSink::interruptProgressIndication() {
@@ -54,8 +55,7 @@ void NiceStderrSink::interruptProgressIndication() {
 
 void NiceStderrSink::resumeProgressIndication() {
 	std::cerr << "Progress (cont'd): ";
-	progressBar = boost::in_place();
-	progressBar->setClearOnDestruction(false);
+	progressBar = boost::in_place(progress);
 }
 
 QuietStderrSink::QuietStderrSink(Level minLevel) :

rhubarb/src/rhubarb/sinks.h

@@ -17,6 +17,7 @@ private:
 	void resumeProgressIndication();
 
 	logging::Level minLevel;
+	double progress;
 	boost::optional<ProgressBar> progressBar;
 	std::shared_ptr<Sink> innerSink;
 };

rhubarb/src/tools/ProgressBar.cpp

@@ -3,55 +3,26 @@
 #include <future>
 #include <chrono>
 #include <format.h>
-#include <iostream>
 #include <boost/algorithm/clamp.hpp>
 #include <cmath>
 
 using std::string;
 
-ProgressForwarder::ProgressForwarder(std::function<void(double progress)> callback) :
-	callback(callback)
+double sanitizeProgress(double progress) {
+	// Make sure value is in [0..1] range
+	return std::isnan(progress)
+		? 0.0
+		: boost::algorithm::clamp(progress, 0.0, 1.0);
+}
+
+ProgressBar::ProgressBar(double progress) :
+	ProgressBar(std::cerr, progress)
 {}
 
-void ProgressForwarder::reportProgress(double value) {
-	callback(value);
-}
-
-ProgressMerger::ProgressMerger(ProgressSink& sink) :
-	sink(sink)
-{}
-
-ProgressSink& ProgressMerger::addSink(double weight) {
-	std::lock_guard<std::mutex> lock(mutex);
-
-	totalWeight += weight;
-	int sinkIndex = weightedValues.size();
-	weightedValues.push_back(0);
-	forwarders.push_back(ProgressForwarder([weight, sinkIndex, this](double progress) {
-		weightedValues[sinkIndex] = progress * weight;
-		report();
-	}));
-	return forwarders.back();
-}
-
-void ProgressMerger::report() {
-	std::lock_guard<std::mutex> lock(mutex);
-
-	if (totalWeight != 0) {
-		double weightedSum = 0;
-		for (double weightedValue : weightedValues) {
-			weightedSum += weightedValue;
-		}
-		double progress = weightedSum / totalWeight;
-		sink.reportProgress(progress);
-	} else {
-		sink.reportProgress(0);
-	}
-}
-
-ProgressBar::ProgressBar(std::ostream& stream) :
+ProgressBar::ProgressBar(std::ostream& stream, double progress) :
 	stream(stream)
 {
+	currentProgress = sanitizeProgress(progress);
 	updateLoopFuture = std::async(std::launch::async, &ProgressBar::updateLoop, this);
 }
 
@@ -61,37 +32,42 @@ ProgressBar::~ProgressBar() {
 }
 
 void ProgressBar::reportProgress(double value) {
-	// Make sure value is in [0..1] range
-	value = boost::algorithm::clamp(value, 0.0, 1.0);
-	if (std::isnan(value)) {
-		value = 0.0;
-	}
-
-	currentProgress = value;
+	currentProgress = sanitizeProgress(value);
 }
 
 void ProgressBar::updateLoop() {
-	const int blockCount = 20;
 	const std::chrono::milliseconds animationInterval(1000 / 8);
-	const string animation = "|/-\\";
 
 	while (!done) {
-		int progressBlockCount = static_cast<int>(currentProgress * blockCount);
-		int percent = static_cast<int>(currentProgress * 100);
-		string text = fmt::format("[{0}{1}] {2:3}% {3}",
-			string(progressBlockCount, '#'), string(blockCount - progressBlockCount, '-'),
-			percent,
-			animation[animationIndex++ % animation.size()]);
-		updateText(text);
-
+		update();
 		std::this_thread::sleep_for(animationInterval);
 	}
 
 	if (clearOnDestruction) {
 		updateText("");
+	} else {
+		update(false);
 	}
 }
 
+void ProgressBar::update(bool showSpinner) {
+	const int blockCount = 20;
+	const string animation = "|/-\\";
+
+	int progressBlockCount = static_cast<int>(currentProgress * blockCount);
+	const double epsilon = 0.0001;
+	int percent = static_cast<int>(currentProgress * 100 + epsilon);
+	const string spinner = showSpinner
+		? string(1, animation[animationIndex++ % animation.size()])
+		: "";
+	string text = fmt::format("[{0}{1}] {2:3}% {3}",
+		string(progressBlockCount, '#'), string(blockCount - progressBlockCount, '-'),
+		percent,
+		spinner
+	);
+	updateText(text);
+}
+
 void ProgressBar::updateText(const string& text) {
 	// Get length of common portion
 	int commonPrefixLength = 0;

rhubarb/src/tools/ProgressBar.h

@@ -2,49 +2,13 @@
 
 #include <atomic>
 #include <future>
-#include <functional>
-#include <list>
-#include <vector>
-#include <mutex>
-#include <string>
 #include <iostream>
-
-class ProgressSink {
-public:
-	virtual ~ProgressSink() {}
-	virtual void reportProgress(double value) = 0;
-};
-
-class NullProgressSink : public ProgressSink {
-public:
-	void reportProgress(double) override {}
-};
-
-class ProgressForwarder : public ProgressSink {
-public:
-	ProgressForwarder(std::function<void(double progress)> callback);
-	void reportProgress(double value) override;
-private:
-	std::function<void(double progress)> callback;
-};
-
-class ProgressMerger {
-public:
-	ProgressMerger(ProgressSink& sink);
-	ProgressSink& addSink(double weight);
-private:
-	void report();
-
-	ProgressSink& sink;
-	std::mutex mutex;
-	double totalWeight = 0;
-	std::list<ProgressForwarder> forwarders;
-	std::vector<double> weightedValues;
-};
+#include "progress.h"
 
 class ProgressBar : public ProgressSink {
 public:
-	ProgressBar(std::ostream& stream = std::cerr);
+	explicit ProgressBar(double progress = 0.0);
+	ProgressBar(std::ostream& stream, double progress = 0.0);
 	~ProgressBar();
 	void reportProgress(double value) override;
 
@@ -58,6 +22,7 @@ public:
 
 private:
 	void updateLoop();
+	void update(bool showSpinner = true);
 	void updateText(const std::string& text);
 
 	std::future<void> updateLoopFuture;
@@ -67,5 +32,5 @@ private:
 	std::ostream& stream;
 	std::string currentText;
 	int animationIndex = 0;
-	bool clearOnDestruction = true;
+	bool clearOnDestruction = false;
 };

rhubarb/src/tools/parallel.h

@@ -1,7 +1,8 @@
 #pragma once
 
 #include <functional>
-#include "ProgressBar.h"
+#include <future>
+#include "progress.h"
 #include <gsl_util.h>
 
 template<typename TCollection>

rhubarb/src/tools/progress.cpp

@@ -0,0 +1,45 @@
+#include "progress.h"
+
+#include <mutex>
+
+using std::string;
+
+ProgressForwarder::ProgressForwarder(std::function<void(double progress)> callback) :
+	callback(callback)
+{}
+
+void ProgressForwarder::reportProgress(double value) {
+	callback(value);
+}
+
+ProgressMerger::ProgressMerger(ProgressSink& sink) :
+	sink(sink)
+{}
+
+ProgressSink& ProgressMerger::addSink(double weight) {
+	std::lock_guard<std::mutex> lock(mutex);
+
+	totalWeight += weight;
+	int sinkIndex = weightedValues.size();
+	weightedValues.push_back(0);
+	forwarders.push_back(ProgressForwarder([weight, sinkIndex, this](double progress) {
+		weightedValues[sinkIndex] = progress * weight;
+		report();
+	}));
+	return forwarders.back();
+}
+
+void ProgressMerger::report() {
+	std::lock_guard<std::mutex> lock(mutex);
+
+	if (totalWeight != 0) {
+		double weightedSum = 0;
+		for (double weightedValue : weightedValues) {
+			weightedSum += weightedValue;
+		}
+		double progress = weightedSum / totalWeight;
+		sink.reportProgress(progress);
+	} else {
+		sink.reportProgress(0);
+	}
+}

rhubarb/src/tools/progress.h

@@ -0,0 +1,39 @@
+#pragma once
+
+#include <list>
+#include <functional>
+#include <mutex>
+#include <vector>
+
+class ProgressSink {
+public:
+	virtual ~ProgressSink() {}
+	virtual void reportProgress(double value) = 0;
+};
+
+class NullProgressSink : public ProgressSink {
+public:
+	void reportProgress(double) override {}
+};
+
+class ProgressForwarder : public ProgressSink {
+public:
+	ProgressForwarder(std::function<void(double progress)> callback);
+	void reportProgress(double value) override;
+private:
+	std::function<void(double progress)> callback;
+};
+
+class ProgressMerger {
+public:
+	ProgressMerger(ProgressSink& sink);
+	ProgressSink& addSink(double weight);
+private:
+	void report();
+
+	ProgressSink& sink;
+	std::mutex mutex;
+	double totalWeight = 0;
+	std::list<ProgressForwarder> forwarders;
+	std::vector<double> weightedValues;
+};