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

14 files changed, 106 insertions, 75 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;
 };
diff --git a/src/wx/content_advanced_dialog.cc b/src/wx/content_advanced_dialog.cc
index 27f34e19a..f3f59eb52 100644
--- a/src/wx/content_advanced_dialog.cc
+++ b/src/wx/content_advanced_dialog.cc
@@ -187,7 +187,7 @@ ContentAdvancedDialog::edit_filters ()
 
 
 void
-ContentAdvancedDialog::filters_changed (vector<Filter const *> filters)
+ContentAdvancedDialog::filters_changed(vector<Filter> const& filters)
 {
 	_filters_list = filters;
 	setup_filters ();
diff --git a/src/wx/content_advanced_dialog.h b/src/wx/content_advanced_dialog.h
index 517ad04e5..8f27fd822 100644
--- a/src/wx/content_advanced_dialog.h
+++ b/src/wx/content_advanced_dialog.h
@@ -42,7 +42,7 @@ public:
 
 	bool ignore_video() const;
 
-	std::vector<Filter const*> filters() {
+	std::vector<Filter> filters() {
 		return _filters_list;
 	}
 
@@ -51,7 +51,7 @@ public:
 
 private:
 	void edit_filters ();
-	void filters_changed (std::vector<Filter const *> filters);
+	void filters_changed(std::vector<Filter> const& filters);
 	void setup_filters ();
 	void set_video_frame_rate ();
 	void video_frame_rate_changed ();
@@ -60,7 +60,7 @@ private:
 
 	std::shared_ptr<Content> _content;
 	bool _filters_allowed = false;
-	std::vector<Filter const*> _filters_list;
+	std::vector<Filter> _filters_list;
 
 	wxStaticText* _filters;
 	wxButton* _filters_button;
diff --git a/src/wx/filter_dialog.cc b/src/wx/filter_dialog.cc
index f9ba06eae..517978c74 100644
--- a/src/wx/filter_dialog.cc
+++ b/src/wx/filter_dialog.cc
@@ -36,7 +36,7 @@ using namespace std;
 using boost::bind;
 
 
-FilterDialog::FilterDialog (wxWindow* parent, vector<Filter const *> const & active)
+FilterDialog::FilterDialog(wxWindow* parent, vector<Filter> const& active)
 	: wxDialog (parent, wxID_ANY, wxString(_("Filters")))
 {
 	auto panel = new wxPanel (this);
@@ -44,29 +44,29 @@ FilterDialog::FilterDialog (wxWindow* parent, vector<Filter const *> const & act
 
 	auto filters = Filter::all ();
 
-	map<string, list<Filter const *>> categories;
+	map<string, list<Filter>> categories;
 
 	for (auto i: filters) {
-		auto j = categories.find (i->category());
+		auto j = categories.find(i.category());
 		if (j == categories.end ()) {
-			categories[i->category()] = { i };
+			categories[i.category()] = { i };
 		} else {
 			j->second.push_back (i);
 		}
 	}
 
-	for (auto const& i: categories) {
-		auto c = new StaticText (panel, std_to_wx(i.first));
+	for (auto const& category: categories) {
+		auto c = new StaticText(panel, std_to_wx(category.first));
 		auto font = c->GetFont();
 		font.SetWeight(wxFONTWEIGHT_BOLD);
 		c->SetFont(font);
 		sizer->Add (c, 1, wxTOP | wxBOTTOM, DCPOMATIC_SIZER_GAP);
 
-		for (auto j: i.second) {
-			auto b = new CheckBox(panel, std_to_wx(j->name()));
-			bool const a = find (active.begin(), active.end(), j) != active.end();
+		for (auto const& filter: category.second) {
+			auto b = new CheckBox(panel, std_to_wx(filter.name()));
+			bool const a = find(active.begin(), active.end(), filter) != active.end();
 			b->SetValue (a);
-			_filters[j] = b;
+			_filters[filter] = b;
 			b->bind(&FilterDialog::filter_toggled, this);
 			sizer->Add (b);
 		}
@@ -95,10 +95,10 @@ FilterDialog::filter_toggled ()
 }
 
 
-vector<Filter const*>
+vector<Filter>
 FilterDialog::active () const
 {
-	vector<Filter const *> active;
+	vector<Filter> active;
 	for (auto const& i: _filters) {
 		if (i.second->IsChecked()) {
 			active.push_back(i.first);
diff --git a/src/wx/filter_dialog.h b/src/wx/filter_dialog.h
index 44dbb502d..aaa43c3e4 100644
--- a/src/wx/filter_dialog.h
+++ b/src/wx/filter_dialog.h
@@ -41,14 +41,14 @@ class Filter;
 class FilterDialog : public wxDialog
 {
 public:
-	FilterDialog (wxWindow *, std::vector<Filter const *> const &);
+	FilterDialog(wxWindow *, std::vector<Filter> const&);
 
-	boost::signals2::signal<void (std::vector<Filter const *>)> ActiveChanged;
+	boost::signals2::signal<void (std::vector<Filter>)> ActiveChanged;
 
 private:
 	void active_changed ();
 	void filter_toggled ();
-	std::vector<Filter const *> active () const;
+	std::vector<Filter> active() const;
 
-	std::map<Filter const *, wxCheckBox *> _filters;
+	std::map<Filter, wxCheckBox *> _filters;
 };
diff --git a/src/wx/simple_video_view.cc b/src/wx/simple_video_view.cc
index 85d6b1036..8524c1fe9 100644
--- a/src/wx/simple_video_view.cc
+++ b/src/wx/simple_video_view.cc
@@ -49,7 +49,7 @@ using namespace dcpomatic;
 SimpleVideoView::SimpleVideoView (FilmViewer* viewer, wxWindow* parent)
 	: VideoView (viewer)
 	, _rec2020_filter("convert", "convert", "", "colorspace=all=bt709:iall=bt2020")
-	, _rec2020_filter_graph({ &_rec2020_filter }, dcp::Fraction(24, 1))
+	, _rec2020_filter_graph({ _rec2020_filter }, dcp::Fraction(24, 1))
 {
 	_panel = new wxPanel (parent);