Cleanup: handle Filter objects by value rather than by reference.

9 files changed, 85 insertions, 54 deletions

diff --git a/src/lib/audio_analyser.cc b/src/lib/audio_analyser.cc
index 5f8f88c5a..45097c5b6 100644
--- a/src/lib/audio_analyser.cc
+++ b/src/lib/audio_analyser.cc
@@ -65,7 +65,7 @@ AudioAnalyser::AudioAnalyser (shared_ptr<const Film> film, shared_ptr<const Play
 {
 
 #ifdef DCPOMATIC_HAVE_EBUR128_PATCHED_FFMPEG
-	_filters.push_back (new Filter("ebur128", "ebur128", "audio", "ebur128=peak=true"));
+	_filters.push_back({"ebur128", "ebur128", "audio", "ebur128=peak=true"});
 	_ebur128.setup(_filters);
 #endif
 
@@ -124,14 +124,6 @@ AudioAnalyser::AudioAnalyser (shared_ptr<const Film> film, shared_ptr<const Play
 }
 
 
-AudioAnalyser::~AudioAnalyser ()
-{
-	for (auto i: _filters) {
-		delete const_cast<Filter*> (i);
-	}
-}
-
-
 void
 AudioAnalyser::analyse (shared_ptr<AudioBuffers> b, DCPTime time)
 {
diff --git a/src/lib/audio_analyser.h b/src/lib/audio_analyser.h
index 9dd92465e..4708f517a 100644
--- a/src/lib/audio_analyser.h
+++ b/src/lib/audio_analyser.h
@@ -40,7 +40,6 @@ class AudioAnalyser
 {
 public:
 	AudioAnalyser (std::shared_ptr<const Film> film, std::shared_ptr<const Playlist> playlist, bool from_zero, std::function<void (float)> set_progress);
-	~AudioAnalyser ();
 
 	AudioAnalyser (AudioAnalyser const&) = delete;
 	AudioAnalyser& operator= (AudioAnalyser const&) = delete;
@@ -67,7 +66,7 @@ private:
 #ifdef DCPOMATIC_HAVE_EBUR128_PATCHED_FFMPEG
 	AudioFilterGraph _ebur128;
 #endif
-	std::vector<Filter const *> _filters;
+	std::vector<Filter> _filters;
 	Frame _samples_per_point = 1;
 
 	boost::scoped_ptr<leqm_nrt::Calculator> _leqm;
diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index 6681a4f0a..ceebb5572 100644
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -115,9 +115,8 @@ FFmpegContent::FFmpegContent (cxml::ConstNodePtr node, int version, list<string>
 	}
 
 	for (auto i: node->node_children("Filter")) {
-		Filter const * f = Filter::from_id(i->content());
-		if (f) {
-			_filters.push_back (f);
+		if (auto filter = Filter::from_id(i->content())) {
+			_filters.push_back(*filter);
 		} else {
 			notes.push_back (String::compose (_("DCP-o-matic no longer supports the `%1' filter, so it has been turned off."), i->content()));
 		}
@@ -232,7 +231,7 @@ FFmpegContent::as_xml (xmlpp::Node* node, bool with_paths) const
 	}
 
 	for (auto i: _filters) {
-		node->add_child("Filter")->add_child_text(i->id());
+		node->add_child("Filter")->add_child_text(i.id());
 	}
 
 	if (_first_video) {
@@ -292,12 +291,12 @@ FFmpegContent::examine (shared_ptr<const Film> film, shared_ptr<Job> job)
 			if (examiner->rotation()) {
 				auto rot = *examiner->rotation ();
 				if (fabs (rot - 180) < 1.0) {
-					_filters.push_back (Filter::from_id ("vflip"));
-					_filters.push_back (Filter::from_id ("hflip"));
+					_filters.push_back(*Filter::from_id("vflip"));
+					_filters.push_back(*Filter::from_id("hflip"));
 				} else if (fabs (rot - 90) < 1.0) {
-					_filters.push_back (Filter::from_id ("90clock"));
+					_filters.push_back(*Filter::from_id("90clock"));
 				} else if (fabs (rot - 270) < 1.0) {
-					_filters.push_back (Filter::from_id ("90anticlock"));
+					_filters.push_back(*Filter::from_id("90anticlock"));
 				}
 			}
 		}
@@ -455,7 +454,7 @@ FFmpegContent::approximate_length () const
 
 
 void
-FFmpegContent::set_filters (vector<Filter const *> const & filters)
+FFmpegContent::set_filters(vector<Filter> const& filters)
 {
 	ContentChangeSignaller cc (this, FFmpegContentProperty::FILTERS);
 
@@ -486,7 +485,7 @@ FFmpegContent::identifier () const
 	}
 
 	for (auto i: _filters) {
-		s += "_" + i->id();
+		s += "_" + i.id();
 	}
 
 	return s;
diff --git a/src/lib/ffmpeg_content.h b/src/lib/ffmpeg_content.h
index ce4a8aa69..a86358b76 100644
--- a/src/lib/ffmpeg_content.h
+++ b/src/lib/ffmpeg_content.h
@@ -18,22 +18,28 @@
 
 */
 
+
 #ifndef DCPOMATIC_FFMPEG_CONTENT_H
 #define DCPOMATIC_FFMPEG_CONTENT_H
 
-#include "content.h"
+
 #include "audio_stream.h"
+#include "content.h"
+#include "filter.h"
+
 
 struct AVFormatContext;
 struct AVStream;
 
-class Filter;
-class FFmpegSubtitleStream;
+
 class FFmpegAudioStream;
+class FFmpegSubtitleStream;
+class Filter;
 class VideoContent;
 struct ffmpeg_pts_offset_test;
 struct audio_sampling_rate_test;
 
+
 class FFmpegContentProperty
 {
 public:
@@ -44,6 +50,7 @@ public:
 	static int const KDM;
 };
 
+
 class FFmpegContent : public Content
 {
 public:
@@ -71,7 +78,7 @@ public:
 
 	void set_default_colour_conversion ();
 
-	void set_filters (std::vector<Filter const *> const &);
+	void set_filters(std::vector<Filter> const&);
 
 	std::vector<std::shared_ptr<FFmpegSubtitleStream>> subtitle_streams () const {
 		boost::mutex::scoped_lock lm (_mutex);
@@ -85,7 +92,7 @@ public:
 
 	std::vector<std::shared_ptr<FFmpegAudioStream>> ffmpeg_audio_streams () const;
 
-	std::vector<Filter const *> filters () const {
+	std::vector<Filter> filters() const {
 		boost::mutex::scoped_lock lm (_mutex);
 		return _filters;
 	}
@@ -109,7 +116,7 @@ private:
 	std::shared_ptr<FFmpegSubtitleStream> _subtitle_stream;
 	boost::optional<dcpomatic::ContentTime> _first_video;
 	/** Video filters that should be used when generating DCPs */
-	std::vector<Filter const *> _filters;
+	std::vector<Filter> _filters;
 
 	boost::optional<AVColorRange> _color_range;
 	boost::optional<AVColorPrimaries> _color_primaries;
diff --git a/src/lib/filter.cc b/src/lib/filter.cc
index 9158cba5c..7db329b00 100644
--- a/src/lib/filter.cc
+++ b/src/lib/filter.cc
@@ -24,6 +24,7 @@
  */
 
 
+#include "dcpomatic_assert.h"
 #include "filter.h"
 #include <dcp/warnings.h>
 LIBDCP_DISABLE_WARNINGS
@@ -31,11 +32,13 @@ extern "C" {
 #include <libavfilter/avfilter.h>
 }
 LIBDCP_ENABLE_WARNINGS
+#include <algorithm>
 
 #include "i18n.h"
 
 
 using namespace std;
+using boost::optional;
 
 
 vector<Filter> Filter::_filters;
@@ -57,14 +60,10 @@ Filter::Filter (string i, string n, string c, string f)
 
 
 /** @return All available filters */
-vector<Filter const *>
+vector<Filter>
 Filter::all ()
 {
-	vector<Filter const *> raw;
-	for (auto& filter: _filters) {
-		raw.push_back (&filter);
-	}
-	return raw;
+	return _filters;
 }
 
 
@@ -113,15 +112,15 @@ Filter::maybe_add (string i, string n, string c, string f)
  *  @return String to pass to FFmpeg for the video filters.
  */
 string
-Filter::ffmpeg_string (vector<Filter const *> const & filters)
+Filter::ffmpeg_string(vector<Filter> const& filters)
 {
 	string ff;
 
-	for (auto const i: filters) {
+	for (auto const& i: filters) {
 		if (!ff.empty ()) {
 			ff += N_(",");
 		}
-		ff += i->ffmpeg ();
+		ff += i.ffmpeg();
 	}
 
 	return ff;
@@ -129,19 +128,47 @@ Filter::ffmpeg_string (vector<Filter const *> const & filters)
 
 
 /** @param d Our id.
- *  @return Corresponding Filter, or 0.
+ *  @return Corresponding Filter, if found.
  */
-Filter const *
-Filter::from_id (string d)
+optional<Filter>
+Filter::from_id(string id)
+{
+	auto iter = std::find_if(_filters.begin(), _filters.end(), [id](Filter const& filter) { return filter.id() == id; });
+	if (iter == _filters.end()) {
+		return {};
+	}
+	return *iter;
+}
+
+
+bool
+operator==(Filter const& a, Filter const& b)
+{
+	return a.id() == b.id() && a.name() == b.name() && a.category() == b.category() && a.ffmpeg() == b.ffmpeg();
+}
+
+
+bool
+operator!=(Filter const& a, Filter const& b)
+{
+	return a.id() != b.id() || a.name() != b.name() || a.category() != b.category() || a.ffmpeg() != b.ffmpeg();
+}
+
+
+bool
+operator<(Filter const& a, Filter const& b)
 {
-	auto i = _filters.begin ();
-	while (i != _filters.end() && i->id() != d) {
-		++i;
+	if (a.id() != b.id()) {
+		return a.id() < b.id();
+	}
+
+	if (a.name() != b.name()) {
+		return a.name() < b.name();
 	}
 
-	if (i == _filters.end ()) {
-		return nullptr;
+	if (a.category() != b.category()) {
+		return a.category() < b.category();
 	}
 
-	return &(*i);
+	return a.ffmpeg() < b.ffmpeg();
 }
diff --git a/src/lib/filter.h b/src/lib/filter.h
index f73a95453..1144a2ca4 100644
--- a/src/lib/filter.h
+++ b/src/lib/filter.h
@@ -28,6 +28,7 @@
 #define DCPOMATIC_FILTER_H
 
 
+#include <boost/optional.hpp>
 #include <string>
 #include <vector>
 
@@ -63,10 +64,10 @@ public:
 		return _category;
 	}
 
-	static std::vector<Filter const *> all ();
-	static Filter const * from_id (std::string d);
+	static std::vector<Filter> all ();
+	static boost::optional<Filter> from_id(std::string d);
 	static void setup_filters ();
-	static std::string ffmpeg_string (std::vector<Filter const *> const & filters);
+	static std::string ffmpeg_string(std::vector<Filter> const& filters);
 
 private:
 
@@ -84,4 +85,9 @@ private:
 };
 
 
+bool operator==(Filter const& a, Filter const& b);
+bool operator!=(Filter const& a, Filter const& b);
+bool operator<(Filter const& a, Filter const& b);
+
+
 #endif
diff --git a/src/lib/filter_graph.cc b/src/lib/filter_graph.cc
index fc6b9033a..2dbb2afc3 100644
--- a/src/lib/filter_graph.cc
+++ b/src/lib/filter_graph.cc
@@ -51,7 +51,7 @@ using dcp::Size;
 
 
 void
-FilterGraph::setup (vector<Filter const *> filters)
+FilterGraph::setup(vector<Filter> const& filters)
 {
 	auto const filters_string = Filter::ffmpeg_string (filters);
 	if (filters.empty()) {
diff --git a/src/lib/filter_graph.h b/src/lib/filter_graph.h
index d56f15296..315aa7835 100644
--- a/src/lib/filter_graph.h
+++ b/src/lib/filter_graph.h
@@ -28,6 +28,7 @@
 #define DCPOMATIC_FILTER_GRAPH_H
 
 
+#include "filter.h"
 #include <dcp/warnings.h>
 LIBDCP_DISABLE_WARNINGS
 extern "C" {
@@ -38,10 +39,10 @@ LIBDCP_ENABLE_WARNINGS
 #include <vector>
 
 
+class Filter;
+class Image;
 struct AVFilterContext;
 struct AVFrame;
-class Image;
-class Filter;
 
 
 /** @class FilterGraph
@@ -56,7 +57,7 @@ public:
 	FilterGraph (FilterGraph const&) = delete;
 	FilterGraph& operator== (FilterGraph const&) = delete;
 
-	void setup (std::vector<Filter const *>);
+	void setup(std::vector<Filter> const&);
 	AVFilterContext* get (std::string name);
 
 protected:
diff --git a/src/lib/video_filter_graph_set.h b/src/lib/video_filter_graph_set.h
index 935378432..f0cbc224f 100644
--- a/src/lib/video_filter_graph_set.h
+++ b/src/lib/video_filter_graph_set.h
@@ -38,7 +38,7 @@ class VideoFilterGraph;
 class VideoFilterGraphSet
 {
 public:
-	VideoFilterGraphSet(std::vector<Filter const*> filters, dcp::Fraction frame_rate)
+	VideoFilterGraphSet(std::vector<Filter> const& filters, dcp::Fraction frame_rate)
 		: _filters(filters)
 		, _frame_rate(frame_rate)
 	{}
@@ -51,7 +51,7 @@ public:
 	void clear();
 
 private:
-	std::vector<Filter const*> _filters;
+	std::vector<Filter> _filters;
 	dcp::Fraction _frame_rate;
 	std::vector<std::shared_ptr<VideoFilterGraph>> _graphs;
 };