Try to fix the filter / AVFrame ownership.

diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc
index cd68e52941c22d6ead11e959928bc3e80442b9fa..982139515b0af588eeb9c8180e5accd74cef444f 100644 (file)
--- a/src/lib/ffmpeg_decoder.cc
+++ b/src/lib/ffmpeg_decoder.cc
@@ -506,12 +506,6 @@ FFmpegDecoder::filter_and_emit_video ()
                return;
        }
        
-       if (_film->crop() == Crop() && _film->filters().empty()) {
-               /* No filter graph needed; just emit */
-               emit_video (shared_ptr<Image> (new FrameImage (_frame, false)), false, bet * av_q2d (_format_context->streams[_video_stream]->time_base));
-               return;
-       }
-       
        shared_ptr<FilterGraph> graph;
 
        {
@@ -523,7 +517,7 @@ FFmpegDecoder::filter_and_emit_video ()
                }
                
                if (i == _filter_graphs.end ()) {
-                       graph.reset (new FilterGraph (_film, this, libdcp::Size (_frame->width, _frame->height), (AVPixelFormat) _frame->format));
+                       graph = filter_graph_factory (_film, this, libdcp::Size (_frame->width, _frame->height), (AVPixelFormat) _frame->format);
                        _filter_graphs.push_back (graph);
                        _film->log()->log (String::compose (N_("New graph for %1x%2, pixel format %3"), _frame->width, _frame->height, _frame->format));
                } else {

diff --git a/src/lib/filter_graph.cc b/src/lib/filter_graph.cc
index 2624bc4d7c0354a9bf46a48348a6c75d38dd5872..b0427a23d0566166921fd6e95eceb5b86c879505 100644 (file)
--- a/src/lib/filter_graph.cc
+++ b/src/lib/filter_graph.cc
@@ -44,18 +44,20 @@ using std::list;
 using boost::shared_ptr;
 using libdcp::Size;
 
-/** Construct a FilterGraph for the settings in a film.
+/** Construct a FFmpegFilterGraph for the settings in a film.
  *  @param film Film.
  *  @param decoder Decoder that we are using.
  *  @param s Size of the images to process.
  *  @param p Pixel format of the images to process.
  */
-FilterGraph::FilterGraph (shared_ptr<Film> film, FFmpegDecoder* decoder, libdcp::Size s, AVPixelFormat p)
+FFmpegFilterGraph::FFmpegFilterGraph (shared_ptr<Film> film, FFmpegDecoder* decoder, libdcp::Size s, AVPixelFormat p)
        : _buffer_src_context (0)
        , _buffer_sink_context (0)
        , _size (s)
        , _pixel_format (p)
 {
+       _frame = av_frame_alloc ();
+       
        string filters = Filter::ffmpeg_strings (film->filters()).first;
        if (!filters.empty ()) {
                filters += N_(",");
@@ -125,11 +127,16 @@ FilterGraph::FilterGraph (shared_ptr<Film> film, FFmpegDecoder* decoder, libdcp:
        /* XXX: leaking `inputs' / `outputs' ? */
 }
 
+FFmpegFilterGraph::~FFmpegFilterGraph ()
+{
+       av_frame_free (&_frame);
+}
+
 /** Take an AVFrame and process it using our configured filters, returning a
- *  set of Images.
+ *  set of Images.  Caller handles memory management of the input frame.
  */
 list<shared_ptr<Image> >
-FilterGraph::process (AVFrame* frame)
+FFmpegFilterGraph::process (AVFrame* frame)
 {
        list<shared_ptr<Image> > images;
 
@@ -138,14 +145,11 @@ FilterGraph::process (AVFrame* frame)
        }
 
        while (1) {
-               AVFrame* frame = av_frame_alloc ();
-               if (av_buffersink_get_frame (_buffer_sink_context, frame) < 0) {
-                       av_frame_free (&frame);
+               if (av_buffersink_get_frame (_buffer_sink_context, _frame) < 0) {
                        break;
                }
 
-               /* This takes ownership of the AVFrame */
-               images.push_back (shared_ptr<Image> (new FrameImage (frame, true)));
+               images.push_back (shared_ptr<Image> (new SimpleImage (_frame)));
        }
        
        return images;
@@ -156,7 +160,25 @@ FilterGraph::process (AVFrame* frame)
  *  @return true if this chain can process images with `s' and `p', otherwise false.
  */
 bool
-FilterGraph::can_process (libdcp::Size s, AVPixelFormat p) const
+FFmpegFilterGraph::can_process (libdcp::Size s, AVPixelFormat p) const
 {
        return (_size == s && _pixel_format == p);
 }
+
+list<shared_ptr<Image> >
+EmptyFilterGraph::process (AVFrame* frame)
+{
+       list<shared_ptr<Image> > im;
+       im.push_back (shared_ptr<Image> (new SimpleImage (frame)));
+       return im;
+}
+
+shared_ptr<FilterGraph>
+filter_graph_factory (shared_ptr<Film> film, FFmpegDecoder* decoder, libdcp::Size size, AVPixelFormat pixel_format)
+{
+       if (film->filters().empty() && film->crop() == Crop()) {
+               return shared_ptr<FilterGraph> (new EmptyFilterGraph);
+       }
+
+       return shared_ptr<FilterGraph> (new FFmpegFilterGraph (film, decoder, size, pixel_format));
+}

diff --git a/src/lib/filter_graph.h b/src/lib/filter_graph.h
index 249b89851bf31ae39ed52bb70f224fad99986e0e..2138943e42b6e3e6ef11ba4967f0c85f0755d618 100644 (file)
--- a/src/lib/filter_graph.h
+++ b/src/lib/filter_graph.h
@@ -30,13 +30,31 @@ class Image;
 class VideoFilter;
 class FFmpegDecoder;
 
-/** @class FilterGraph
+class FilterGraph
+{
+public:
+       virtual bool can_process (libdcp::Size, AVPixelFormat) const = 0;
+       virtual std::list<boost::shared_ptr<Image> > process (AVFrame *) = 0;
+};
+
+class EmptyFilterGraph : public FilterGraph
+{
+public:
+       bool can_process (libdcp::Size, AVPixelFormat) const {
+               return true;
+       }
+
+       std::list<boost::shared_ptr<Image> > process (AVFrame *);
+};
+
+/** @class FFmpegFilterGraph
  *  @brief A graph of FFmpeg filters.
  */
-class FilterGraph
+class FFmpegFilterGraph : public FilterGraph
 {
 public:
-       FilterGraph (boost::shared_ptr<Film> film, FFmpegDecoder* decoder, libdcp::Size s, AVPixelFormat p);
+       FFmpegFilterGraph (boost::shared_ptr<Film> film, FFmpegDecoder* decoder, libdcp::Size s, AVPixelFormat p);
+       ~FFmpegFilterGraph ();
 
        bool can_process (libdcp::Size s, AVPixelFormat p) const;
        std::list<boost::shared_ptr<Image> > process (AVFrame * frame);
@@ -46,6 +64,9 @@ private:
        AVFilterContext* _buffer_sink_context;
        libdcp::Size _size; ///< size of the images that this chain can process
        AVPixelFormat _pixel_format; ///< pixel format of the images that this chain can process
+       AVFrame* _frame;
 };
 
+boost::shared_ptr<FilterGraph> filter_graph_factory (boost::shared_ptr<Film>, FFmpegDecoder *, libdcp::Size, AVPixelFormat);
+
 #endif

diff --git a/src/lib/image.cc b/src/lib/image.cc
index 1768be924fe806d6b6c1247cf38f9086572c29aa..b166dfac6a0c72054531dc19cce0b9a7fd445971 100644 (file)
--- a/src/lib/image.cc
+++ b/src/lib/image.cc
@@ -469,10 +469,9 @@ SimpleImage::allocate ()
 
 SimpleImage::SimpleImage (SimpleImage const & other)
        : Image (other)
+       , _size (other._size)
+       , _aligned (other._aligned)
 {
-       _size = other._size;
-       _aligned = other._aligned;
-       
        allocate ();
 
        for (int i = 0; i < components(); ++i) {
@@ -486,6 +485,25 @@ SimpleImage::SimpleImage (SimpleImage const & other)
        }
 }
 
+SimpleImage::SimpleImage (AVFrame* frame)
+       : Image (static_cast<AVPixelFormat> (frame->format))
+       , _size (frame->width, frame->height)
+       , _aligned (true)
+{
+       allocate ();
+
+       for (int i = 0; i < components(); ++i) {
+               uint8_t* p = _data[i];
+               uint8_t* q = frame->data[i];
+               for (int j = 0; j < lines(i); ++j) {
+                       memcpy (p, q, _line_size[i]);
+                       p += stride()[i];
+                       /* AVFrame's linesize is what we call `stride' */
+                       q += frame->linesize[i];
+               }
+       }
+}
+
 SimpleImage::SimpleImage (shared_ptr<const Image> other)
        : Image (*other.get())
 {
@@ -576,59 +594,6 @@ SimpleImage::aligned () const
        return _aligned;
 }
 
-FrameImage::FrameImage (AVFrame* frame, bool own)
-       : Image (static_cast<AVPixelFormat> (frame->format))
-       , _frame (frame)
-       , _own (own)
-{
-       _line_size = (int *) av_malloc (4 * sizeof (int));
-       _line_size[0] = _line_size[1] = _line_size[2] = _line_size[3] = 0;
-       
-       for (int i = 0; i < components(); ++i) {
-               _line_size[i] = size().width * bytes_per_pixel(i);
-       }
-}
-
-FrameImage::~FrameImage ()
-{
-       if (_own) {
-               av_frame_free (&_frame);
-       }
-       
-       av_free (_line_size);
-}
-
-uint8_t **
-FrameImage::data () const
-{
-       return _frame->data;
-}
-
-int *
-FrameImage::line_size () const
-{
-       return _line_size;
-}
-
-int *
-FrameImage::stride () const
-{
-       /* AVFrame's `linesize' is what we call `stride' */
-       return _frame->linesize;
-}
-
-libdcp::Size
-FrameImage::size () const
-{
-       return libdcp::Size (_frame->width, _frame->height);
-}
-
-bool
-FrameImage::aligned () const
-{
-       return true;
-}
-
 RGBPlusAlphaImage::RGBPlusAlphaImage (shared_ptr<const Image> im)
        : SimpleImage (im->pixel_format(), im->size(), false)
 {

diff --git a/src/lib/image.h b/src/lib/image.h
index 16fbd28c24b93c4064045368cf87e171444c16c9..70dacfaee4bd9e4bcd24bdf8ba4241916b1941e4 100644 (file)
--- a/src/lib/image.h
+++ b/src/lib/image.h
@@ -34,7 +34,6 @@ extern "C" {
 #include "util.h"
 
 class Scaler;
-class RGBFrameImage;
 class SimpleImage;
 
 /** @class Image
@@ -100,31 +99,6 @@ private:
        AVPixelFormat _pixel_format; ///< FFmpeg's way of describing the pixel format of this Image
 };
 
-/** @class FrameImage
- *  @brief An Image that is held in an AVFrame.
- */
-class FrameImage : public Image
-{
-public:
-       FrameImage (AVFrame *, bool);
-       ~FrameImage ();
-
-       uint8_t ** data () const;
-       int * line_size () const;
-       int * stride () const;
-       libdcp::Size size () const;
-       bool aligned () const;
-
-private:
-       /* Not allowed */
-       FrameImage (FrameImage const &);
-       FrameImage& operator= (FrameImage const &);
-       
-       AVFrame* _frame;
-       bool _own;
-       int* _line_size;
-};
-
 /** @class SimpleImage
  *  @brief An Image for which memory is allocated using a `simple' av_malloc().
  */
@@ -132,6 +106,7 @@ class SimpleImage : public Image
 {
 public:
        SimpleImage (AVPixelFormat, libdcp::Size, bool);
+       SimpleImage (AVFrame *);
        SimpleImage (SimpleImage const &);
        SimpleImage (boost::shared_ptr<const Image>);
        SimpleImage& operator= (SimpleImage const &);

diff --git a/test/pixel_formats_test.cc b/test/pixel_formats_test.cc
index e8ad725ff5f85b83cbfa7152eefabecd97051b51..84f2a33ce325209614ae519a36a5fc4113104dab 100644 (file)
--- a/test/pixel_formats_test.cc
+++ b/test/pixel_formats_test.cc
@@ -65,7 +65,7 @@ BOOST_AUTO_TEST_CASE (pixel_formats_test)
                f->width = 640;
                f->height = 480;
                f->format = static_cast<int> (i->format);
-               FrameImage t (f, true);
+               SimpleImage t (f);
                BOOST_CHECK_EQUAL(t.components(), i->components);
                BOOST_CHECK_EQUAL(t.lines(0), i->lines[0]);
                BOOST_CHECK_EQUAL(t.lines(1), i->lines[1]);