From: Carl Hetherington Date: Fri, 3 May 2013 09:01:36 +0000 (+0100) Subject: Try to fix the filter / AVFrame ownership. X-Git-Tag: v2.0.48~1337^2~347^2~3 X-Git-Url: https://git.carlh.net/gitweb/?a=commitdiff_plain;h=3fc3aad8735903ced3dae65f764eb33e3f5b3f11;p=dcpomatic.git Try to fix the filter / AVFrame ownership. --- diff --git a/src/lib/ffmpeg_decoder.cc b/src/lib/ffmpeg_decoder.cc index cd68e5294..982139515 100644 --- 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 (new FrameImage (_frame, false)), false, bet * av_q2d (_format_context->streams[_video_stream]->time_base)); - return; - } - shared_ptr 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 2624bc4d7..b0427a23d 100644 --- 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, FFmpegDecoder* decoder, libdcp::Size s, AVPixelFormat p) +FFmpegFilterGraph::FFmpegFilterGraph (shared_ptr 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, 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 > -FilterGraph::process (AVFrame* frame) +FFmpegFilterGraph::process (AVFrame* frame) { list > 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 (new FrameImage (frame, true))); + images.push_back (shared_ptr (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 > +EmptyFilterGraph::process (AVFrame* frame) +{ + list > im; + im.push_back (shared_ptr (new SimpleImage (frame))); + return im; +} + +shared_ptr +filter_graph_factory (shared_ptr film, FFmpegDecoder* decoder, libdcp::Size size, AVPixelFormat pixel_format) +{ + if (film->filters().empty() && film->crop() == Crop()) { + return shared_ptr (new EmptyFilterGraph); + } + + return shared_ptr (new FFmpegFilterGraph (film, decoder, size, pixel_format)); +} diff --git a/src/lib/filter_graph.h b/src/lib/filter_graph.h index 249b89851..2138943e4 100644 --- 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 > process (AVFrame *) = 0; +}; + +class EmptyFilterGraph : public FilterGraph +{ +public: + bool can_process (libdcp::Size, AVPixelFormat) const { + return true; + } + + std::list > process (AVFrame *); +}; + +/** @class FFmpegFilterGraph * @brief A graph of FFmpeg filters. */ -class FilterGraph +class FFmpegFilterGraph : public FilterGraph { public: - FilterGraph (boost::shared_ptr film, FFmpegDecoder* decoder, libdcp::Size s, AVPixelFormat p); + FFmpegFilterGraph (boost::shared_ptr film, FFmpegDecoder* decoder, libdcp::Size s, AVPixelFormat p); + ~FFmpegFilterGraph (); bool can_process (libdcp::Size s, AVPixelFormat p) const; std::list > 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 filter_graph_factory (boost::shared_ptr, FFmpegDecoder *, libdcp::Size, AVPixelFormat); + #endif diff --git a/src/lib/image.cc b/src/lib/image.cc index 1768be924..b166dfac6 100644 --- 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 (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 other) : Image (*other.get()) { @@ -576,59 +594,6 @@ SimpleImage::aligned () const return _aligned; } -FrameImage::FrameImage (AVFrame* frame, bool own) - : Image (static_cast (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 im) : SimpleImage (im->pixel_format(), im->size(), false) { diff --git a/src/lib/image.h b/src/lib/image.h index 16fbd28c2..70dacfaee 100644 --- 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); SimpleImage& operator= (SimpleImage const &); diff --git a/test/pixel_formats_test.cc b/test/pixel_formats_test.cc index e8ad725ff..84f2a33ce 100644 --- 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 (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]);