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

diff --git a/src/lib/audio_analyser.cc b/src/lib/audio_analyser.cc
index 5f8f88c5adfaa96cb63088819bd31549b82f92e0..45097c5b619cdcab5d296da3c963aaf7632763c0 100644 (file)
--- 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 9dd92465e4c94aa4d0f608de543ac1576b3ddfdc..4708f517a97cfd7d55d5678e6325e7cb76b0ea70 100644 (file)
--- 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 6681a4f0a897aa6c6c9de3c192c1cad434d69a8c..ceebb5572ed03df1a176629eec4d4f940d236482 100644 (file)
--- 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 ce4a8aa6985cdbdc38af33bf5e4c8b479cbc65db..a86358b7677ec46eb07f143585c1cfccf2788647 100644 (file)
--- 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 9158cba5c10b6741995433ea767c2a3899611920..7db329b00fca5c46b13c867ae17caf3337ccea59 100644 (file)
--- 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 f73a954538350d18a287582eddc643996baa96d5..1144a2ca4b0c93efed7c826fca7a253f944de0d5 100644 (file)
--- 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 fc6b9033ac97805e01b4ea45aaeca6dc6fb41208..2dbb2afc3e3f4abd147e6af8936f546df857848a 100644 (file)
--- 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 d56f15296bf608d6daf8c30ff8f576fa8c372f9d..315aa7835c8a108b016b9109947f2f230c6a30bc 100644 (file)
--- 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 935378432fc663316f9fc2cf9464af1a33b9d1db..f0cbc224ff32dddffb58dc4657908f5337a37452 100644 (file)
--- 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 27f34e19afeba1e89b8d613386ae482b2722bee9..f3f59eb52778d2148fc5d2570da4e4374c4739b4 100644 (file)
--- 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 517ad04e538426a73eac4068ef98a0b1b495e028..8f27fd822d89a4439c328bbfb3b80eed612dcc7d 100644 (file)
--- 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 f9ba06eae9b752ff7e98855ed6e55e907b4617f2..517978c746b145040422f86706c25ae224bd848a 100644 (file)
--- 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 44dbb502dd1856e3247f533b5ce193454441523a..aaa43c3e44962818ab8fe0471ad5f5b62d39d4ee 100644 (file)
--- 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 85d6b103668c7240bebbab37483231c409412832..8524c1fe91f559f2b3d1c78a0fe923fee6e8546d 100644 (file)
--- 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);