Fix crash due to writing off the end of an Image's buffer; see comments.

diff --git a/ChangeLog b/ChangeLog
index 0735b47e257d608d56359a52de3f8b2f08f06390..bac567991efa64f58fffeb14ed90b007e7ab8f6a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2015-11-17  c.hetherington  <cth@carlh.net>
+
+       * Fix crash when previewing MXFs in some cases.
+
 2015-11-16  Carl Hetherington  <cth@carlh.net>
 
        * Updated nl_NL translation from Rob van Nieuwkerk.

diff --git a/src/lib/image.cc b/src/lib/image.cc
index 6835d0c26d967557df34fadfa06ba1e9680da53f..37aa2b5e363e1a7545ca25e21ec60959d62f2635 100644 (file)
--- a/src/lib/image.cc
+++ b/src/lib/image.cc
@@ -124,8 +124,29 @@ Image::crop_scale_window (
        DCPOMATIC_ASSERT (out_size.width >= inter_size.width);
        DCPOMATIC_ASSERT (out_size.height >= inter_size.height);
 
-       /* Here's an image of out_size */
-       shared_ptr<Image> out (new Image (out_format, out_size, out_aligned));
+       /* Here's an image of out_size.  Below we may write to it starting at an offset so we get some padding.
+          Hence we want to write in the following pattern:
+
+          block start   write start                                  line end
+          |..(padding)..|<------line-size------------->|..(padding)..|
+          |..(padding)..|<------line-size------------->|..(padding)..|
+          |..(padding)..|<------line-size------------->|..(padding)..|
+
+          where line-size is of the smaller (inter_size) image and the full padded line length is that of
+          out_size.  To get things to work we have to tell FFmpeg that the stride is that of out_size.
+          However some parts of FFmpeg (notably rgb48Toxyz12 in swscale.c) process data for the full
+          specified *stride*.  This does not matter until we get to the last line:
+
+          block start   write start                                  line end
+          |..(padding)..|<------line-size------------->|XXXwrittenXXX|
+          |XXXwrittenXXX|<------line-size------------->|XXXwrittenXXX|
+          |XXXwrittenXXX|<------line-size------------->|XXXwrittenXXXXXXwrittenXXX
+                                                                      ^^^^ out of bounds
+
+          To get around this, we ask Image to overallocate its buffers by the overrun.
+       */
+
+       shared_ptr<Image> out (new Image (out_format, out_size, out_aligned, (out_size.width - inter_size.width) / 2));
        out->make_black ();
 
        /* Size of the image after any crop */
@@ -584,11 +605,13 @@ Image::bytes_per_pixel (int c) const
  *
  *  @param p Pixel format.
  *  @param s Size in pixels.
+ *  @param extra_pixels Amount of extra "run-off" memory to allocate at the end of each plane in pixels.
  */
-Image::Image (AVPixelFormat p, dcp::Size s, bool aligned)
+Image::Image (AVPixelFormat p, dcp::Size s, bool aligned, int extra_pixels)
        : _size (s)
        , _pixel_format (p)
        , _aligned (aligned)
+       , _extra_pixels (extra_pixels)
 {
        allocate ();
 }
@@ -623,7 +646,7 @@ Image::allocate ()
                   so I'll just over-allocate by 32 bytes and have done with it.  Empirical
                   testing suggests that it works.
                */
-               _data[i] = (uint8_t *) wrapped_av_malloc (_stride[i] * sample_size(i).height + 32);
+               _data[i] = (uint8_t *) wrapped_av_malloc (_stride[i] * sample_size(i).height + _extra_pixels * bytes_per_pixel(i) + 32);
        }
 }
 
@@ -631,6 +654,7 @@ Image::Image (Image const & other)
        : _size (other._size)
        , _pixel_format (other._pixel_format)
        , _aligned (other._aligned)
+       , _extra_pixels (other._extra_pixels)
 {
        allocate ();
 
@@ -650,6 +674,7 @@ Image::Image (AVFrame* frame)
        : _size (frame->width, frame->height)
        , _pixel_format (static_cast<AVPixelFormat> (frame->format))
        , _aligned (true)
+       , _extra_pixels (0)
 {
        allocate ();
 
@@ -670,6 +695,7 @@ Image::Image (shared_ptr<const Image> other, bool aligned)
        : _size (other->_size)
        , _pixel_format (other->_pixel_format)
        , _aligned (aligned)
+       , _extra_pixels (other->_extra_pixels)
 {
        allocate ();
 

diff --git a/src/lib/image.h b/src/lib/image.h
index 2d267f861e72a64cd337036bfe815943350a077a..416ee3a8d8fd25b0f5cca0176a9e53af48af7085 100644 (file)
--- a/src/lib/image.h
+++ b/src/lib/image.h
@@ -39,7 +39,7 @@ class Socket;
 class Image
 {
 public:
-       Image (AVPixelFormat, dcp::Size, bool);
+       Image (AVPixelFormat, dcp::Size, bool, int extra_pixels = 0);
        Image (AVFrame *);
        Image (Image const &);
        Image (boost::shared_ptr<const Image>, bool);
@@ -88,6 +88,7 @@ private:
        int* _line_size; ///< array of sizes of the data in each line, in pixels (without any alignment padding bytes)
        int* _stride; ///< array of strides for each line (including any alignment padding bytes)
        bool _aligned;
+       int _extra_pixels;
 };
 
 extern PositionImage merge (std::list<PositionImage> images);