Fix incorrect images when cropping without stretch.
[dcpomatic.git] / src / lib / image.cc
index f005e3f6397b2726291004cd82cbaa1dc3ef7811..1439d9f7f964228385ba20e28c0bbb8ba85082c5 100644 (file)
@@ -140,29 +140,7 @@ 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.  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));
+       shared_ptr<Image> out (new Image(out_format, out_size, out_aligned));
        out->make_black ();
 
        /* Size of the image after any crop */
@@ -245,6 +223,14 @@ Image::crop_scale_window (
 
        sws_freeContext (scale_context);
 
+       if (crop != Crop() && cropped_size == inter_size && _pixel_format == out_format) {
+               /* We are cropping without any scaling or pixel format conversion, so FFmpeg may have left some
+                  data behind in our image.  Clear it out.  It may get to the point where we should just stop
+                  trying to be clever with cropping.
+               */
+               out->make_part_black (corner.x + cropped_size.width, out_size.width - cropped_size.width);
+       }
+
        return out;
 }
 
@@ -342,6 +328,36 @@ Image::swap_16 (uint16_t v)
        return ((v >> 8) & 0xff) | ((v & 0xff) << 8);
 }
 
+void
+Image::make_part_black (int x, int w)
+{
+       switch (_pixel_format) {
+       case AV_PIX_FMT_RGB24:
+       case AV_PIX_FMT_ARGB:
+       case AV_PIX_FMT_RGBA:
+       case AV_PIX_FMT_ABGR:
+       case AV_PIX_FMT_BGRA:
+       case AV_PIX_FMT_RGB555LE:
+       case AV_PIX_FMT_RGB48LE:
+       case AV_PIX_FMT_RGB48BE:
+       case AV_PIX_FMT_XYZ12LE:
+       {
+               int const h = sample_size(0).height;
+               int const bpp = bytes_per_pixel(0);
+               int const s = stride()[0];
+               uint8_t* p = data()[0];
+               for (int y = 0; y < h; y++) {
+                       memset (p + x * bpp, 0, w * bpp);
+                       p += s;
+               }
+               break;
+       }
+
+       default:
+               throw PixelFormatError ("make_part_black()", _pixel_format);
+       }
+}
+
 void
 Image::make_black ()
 {
@@ -469,7 +485,7 @@ Image::make_black ()
 void
 Image::make_transparent ()
 {
-       if (_pixel_format != AV_PIX_FMT_BGRA) {
+       if (_pixel_format != AV_PIX_FMT_BGRA && _pixel_format != AV_PIX_FMT_RGBA) {
                throw PixelFormatError ("make_transparent()", _pixel_format);
        }
 
@@ -815,13 +831,11 @@ Image::bytes_per_pixel (int c) const
  *  @param p Pixel format.
  *  @param s Size in pixels.
  *  @param aligned true to make each row of this image aligned to a 32-byte boundary.
- *  @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, int extra_pixels)
+Image::Image (AVPixelFormat p, dcp::Size s, bool aligned)
        : _size (s)
        , _pixel_format (p)
        , _aligned (aligned)
-       , _extra_pixels (extra_pixels)
 {
        allocate ();
 }
@@ -855,13 +869,36 @@ Image::allocate ()
                   over-reads by more then _avx.  I can't follow the code to work out how much,
                   so I'll just over-allocate by 32 bytes and have done with it.  Empirical
                   testing suggests that it works.
+
+                  In addition to these concerns, we may read/write as much as a whole extra line
+                  at the end of each plane in cases where we are messing with offsets in order to
+                  do pad or crop.  To solve this we over-allocate by an extra _stride[i] bytes.
+
+                  As an example: we may write to images 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
                */
-               _data[i] = (uint8_t *) wrapped_av_malloc (_stride[i] * sample_size(i).height + _extra_pixels * bytes_per_pixel(i) + 32);
+               _data[i] = (uint8_t *) wrapped_av_malloc (_stride[i] * (sample_size(i).height + 1) + 32);
 #if HAVE_VALGRIND_MEMCHECK_H
                /* The data between the end of the line size and the stride is undefined but processed by
                   libswscale, causing lots of valgrind errors.  Mark it all defined to quell these errors.
                */
-               VALGRIND_MAKE_MEM_DEFINED (_data[i], _stride[i] * sample_size(i).height + _extra_pixels * bytes_per_pixel(i) + 32);
+               VALGRIND_MAKE_MEM_DEFINED (_data[i], _stride[i] * (sample_size(i).height + 1) + 32);
 #endif
        }
 }
@@ -871,7 +908,6 @@ Image::Image (Image const & other)
        , _size (other._size)
        , _pixel_format (other._pixel_format)
        , _aligned (other._aligned)
-       , _extra_pixels (other._extra_pixels)
 {
        allocate ();
 
@@ -891,7 +927,6 @@ Image::Image (AVFrame* frame)
        : _size (frame->width, frame->height)
        , _pixel_format (static_cast<AVPixelFormat> (frame->format))
        , _aligned (true)
-       , _extra_pixels (0)
 {
        allocate ();
 
@@ -912,7 +947,6 @@ 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 ();
 
@@ -954,7 +988,6 @@ Image::swap (Image & other)
        }
 
        std::swap (_aligned, other._aligned);
-       std::swap (_extra_pixels, other._extra_pixels);
 }
 
 /** Destroy a Image */
@@ -1058,49 +1091,61 @@ operator== (Image const & a, Image const & b)
 void
 Image::fade (float f)
 {
+       /* U/V black value for 8-bit colour */
+       static int const eight_bit_uv =    (1 << 7) - 1;
+       /* U/V black value for 10-bit colour */
+       static uint16_t const ten_bit_uv = (1 << 9) - 1;
+
        switch (_pixel_format) {
        case AV_PIX_FMT_YUV420P:
-       case AV_PIX_FMT_YUV422P:
-       case AV_PIX_FMT_YUV444P:
-       case AV_PIX_FMT_YUV411P:
-       case AV_PIX_FMT_YUVJ420P:
-       case AV_PIX_FMT_YUVJ422P:
-       case AV_PIX_FMT_YUVJ444P:
-       case AV_PIX_FMT_RGB24:
-       case AV_PIX_FMT_ARGB:
-       case AV_PIX_FMT_RGBA:
-       case AV_PIX_FMT_ABGR:
-       case AV_PIX_FMT_BGRA:
-       case AV_PIX_FMT_RGB555LE:
-               /* 8-bit */
-               for (int c = 0; c < 3; ++c) {
+       {
+               /* Y */
+               uint8_t* p = data()[0];
+               int const lines = sample_size(0).height;
+               for (int y = 0; y < lines; ++y) {
+                       uint8_t* q = p;
+                       for (int x = 0; x < line_size()[0]; ++x) {
+                               *q = int(float(*q) * f);
+                               ++q;
+                       }
+                       p += stride()[0];
+               }
+
+               /* U, V */
+               for (int c = 1; c < 3; ++c) {
                        uint8_t* p = data()[c];
                        int const lines = sample_size(c).height;
                        for (int y = 0; y < lines; ++y) {
                                uint8_t* q = p;
                                for (int x = 0; x < line_size()[c]; ++x) {
-                                       *q = int (float (*q) * f);
+                                       *q = eight_bit_uv + int((int(*q) - eight_bit_uv) * f);
                                        ++q;
                                }
                                p += stride()[c];
                        }
                }
+
                break;
+       }
+
+       case AV_PIX_FMT_RGB24:
+       {
+               /* 8-bit */
+               uint8_t* p = data()[0];
+               int const lines = sample_size(0).height;
+               for (int y = 0; y < lines; ++y) {
+                       uint8_t* q = p;
+                       for (int x = 0; x < line_size()[0]; ++x) {
+                               *q = int (float (*q) * f);
+                               ++q;
+                       }
+                       p += stride()[0];
+               }
+               break;
+       }
 
-       case AV_PIX_FMT_YUV422P9LE:
-       case AV_PIX_FMT_YUV444P9LE:
-       case AV_PIX_FMT_YUV422P10LE:
-       case AV_PIX_FMT_YUV444P10LE:
-       case AV_PIX_FMT_YUV422P16LE:
-       case AV_PIX_FMT_YUV444P16LE:
-       case AV_PIX_FMT_YUVA420P9LE:
-       case AV_PIX_FMT_YUVA422P9LE:
-       case AV_PIX_FMT_YUVA444P9LE:
-       case AV_PIX_FMT_YUVA420P10LE:
-       case AV_PIX_FMT_YUVA422P10LE:
-       case AV_PIX_FMT_YUVA444P10LE:
-       case AV_PIX_FMT_RGB48LE:
        case AV_PIX_FMT_XYZ12LE:
+       case AV_PIX_FMT_RGB48LE:
                /* 16-bit little-endian */
                for (int c = 0; c < 3; ++c) {
                        int const stride_pixels = stride()[c] / 2;
@@ -1118,22 +1163,26 @@ Image::fade (float f)
                }
                break;
 
-       case AV_PIX_FMT_YUV422P9BE:
-       case AV_PIX_FMT_YUV444P9BE:
-       case AV_PIX_FMT_YUV444P10BE:
-       case AV_PIX_FMT_YUV422P10BE:
-       case AV_PIX_FMT_YUVA420P9BE:
-       case AV_PIX_FMT_YUVA422P9BE:
-       case AV_PIX_FMT_YUVA444P9BE:
-       case AV_PIX_FMT_YUVA420P10BE:
-       case AV_PIX_FMT_YUVA422P10BE:
-       case AV_PIX_FMT_YUVA444P10BE:
-       case AV_PIX_FMT_YUVA420P16BE:
-       case AV_PIX_FMT_YUVA422P16BE:
-       case AV_PIX_FMT_YUVA444P16BE:
-       case AV_PIX_FMT_RGB48BE:
-               /* 16-bit big-endian */
-               for (int c = 0; c < 3; ++c) {
+       case AV_PIX_FMT_YUV422P10LE:
+       {
+               /* Y */
+               {
+                       int const stride_pixels = stride()[0] / 2;
+                       int const line_size_pixels = line_size()[0] / 2;
+                       uint16_t* p = reinterpret_cast<uint16_t*> (data()[0]);
+                       int const lines = sample_size(0).height;
+                       for (int y = 0; y < lines; ++y) {
+                               uint16_t* q = p;
+                               for (int x = 0; x < line_size_pixels; ++x) {
+                                       *q = int(float(*q) * f);
+                                       ++q;
+                               }
+                               p += stride_pixels;
+                       }
+               }
+
+               /* U, V */
+               for (int c = 1; c < 3; ++c) {
                        int const stride_pixels = stride()[c] / 2;
                        int const line_size_pixels = line_size()[c] / 2;
                        uint16_t* p = reinterpret_cast<uint16_t*> (data()[c]);
@@ -1141,7 +1190,7 @@ Image::fade (float f)
                        for (int y = 0; y < lines; ++y) {
                                uint16_t* q = p;
                                for (int x = 0; x < line_size_pixels; ++x) {
-                                       *q = swap_16 (int (float (swap_16 (*q)) * f));
+                                       *q = ten_bit_uv + int((int(*q) - ten_bit_uv) * f);
                                        ++q;
                                }
                                p += stride_pixels;
@@ -1149,18 +1198,6 @@ Image::fade (float f)
                }
                break;
 
-       case AV_PIX_FMT_UYVY422:
-       {
-               int const Y = sample_size(0).height;
-               int const X = line_size()[0];
-               uint8_t* p = data()[0];
-               for (int y = 0; y < Y; ++y) {
-                       for (int x = 0; x < X; ++x) {
-                               *p = int (float (*p) * f);
-                               ++p;
-                       }
-               }
-               break;
        }
 
        default:
@@ -1248,7 +1285,9 @@ Image::as_png () const
 {
        DCPOMATIC_ASSERT (bytes_per_pixel(0) == 4);
        DCPOMATIC_ASSERT (planes() == 1);
-       DCPOMATIC_ASSERT (pixel_format() == AV_PIX_FMT_BGRA);
+       if (pixel_format() != AV_PIX_FMT_RGBA) {
+               return convert_pixel_format(dcp::YUV_TO_RGB_REC709, AV_PIX_FMT_RGBA, true, false)->as_png();
+       }
 
        /* error handling? */
        png_structp png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, reinterpret_cast<void*>(const_cast<Image*>(this)), png_error_fn, 0);