Account for J2K decoding at lower-than-maximum resolution when cropping

the resulting images; fixes #1274.

diff --git a/ChangeLog b/ChangeLog
index ded429291cfa56afeec8b0e5c3679c26883bfec3..522e56d8a5c98d4180bc58ddf7f4cd20b90051bd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2018-04-13  Carl Hetherington  <cth@carlh.net>
 
+       * Fix incorrect preview crop with DCP sources when the preview is smaller than half
+       of the DCP's resolution (#1274).
+
        * Update encoding server list when one goes away (#1176).
 
        * Add servers with bad server-link versions in the list (#982).

diff --git a/src/lib/image_proxy.h b/src/lib/image_proxy.h
index 01cb79552d26de566d6ea3ad667c8a819c5618f8..846aec136f3348c0457c5007b12a70df34122458 100644 (file)
--- a/src/lib/image_proxy.h
+++ b/src/lib/image_proxy.h
@@ -63,9 +63,11 @@ public:
        /** @param note Handler for any notes that occur.
         *  @param size Size that the returned image will be scaled to, in case this
         *  can be used as an optimisation.
-        *  @return Image (which must be aligned)
+        *  @return Image (which must be aligned) and log2 of any scaling down that has
+        *  already been applied to the image; e.g. if the the image is already half the size
+        *  of the original, the second part of the return value will be 1.
         */
-       virtual boost::shared_ptr<Image> image (
+       virtual std::pair<boost::shared_ptr<Image>, int> image (
                boost::optional<dcp::NoteHandler> note = boost::optional<dcp::NoteHandler> (),
                boost::optional<dcp::Size> size = boost::optional<dcp::Size> ()
                ) const = 0;
@@ -76,8 +78,9 @@ public:
        virtual bool same (boost::shared_ptr<const ImageProxy>) const = 0;
        /** Do any useful work that would speed up a subsequent call to ::image().
         *  This method may be called in a different thread to image().
+        *  @return log2 of any scaling down that will be applied to the image.
         */
-       virtual void prepare (boost::optional<dcp::Size> = boost::optional<dcp::Size>()) const {}
+       int prepare (boost::optional<dcp::Size> = boost::optional<dcp::Size>()) const { return 0; }
        virtual AVPixelFormat pixel_format () const = 0;
        virtual size_t memory_used () const = 0;
 };

diff --git a/src/lib/j2k_image_proxy.cc b/src/lib/j2k_image_proxy.cc
index 2489a949925f12ffd535779c0efc2507257d0135..9100414e442760ce1c532026afbd5b945e367e5c 100644 (file)
--- a/src/lib/j2k_image_proxy.cc
+++ b/src/lib/j2k_image_proxy.cc
@@ -21,6 +21,7 @@
 #include "j2k_image_proxy.h"
 #include "dcpomatic_socket.h"
 #include "image.h"
+#include "dcpomatic_assert.h"
 #include <dcp/raw_convert.h>
 #include <dcp/openjpeg_image.h>
 #include <dcp/mono_picture_frame.h>
@@ -38,6 +39,8 @@
 using std::string;
 using std::cout;
 using std::max;
+using std::pair;
+using std::make_pair;
 using boost::shared_ptr;
 using boost::optional;
 using boost::dynamic_pointer_cast;
@@ -106,13 +109,14 @@ J2KImageProxy::J2KImageProxy (shared_ptr<cxml::Node> xml, shared_ptr<Socket> soc
        socket->read (_data.data().get (), _data.size ());
 }
 
-void
+int
 J2KImageProxy::prepare (optional<dcp::Size> target_size) const
 {
        boost::mutex::scoped_lock lm (_mutex);
 
        if (_decompressed && target_size == _target_size) {
-               return;
+               DCPOMATIC_ASSERT (_reduce);
+               return *_reduce;
        }
 
        int reduce = 0;
@@ -143,12 +147,15 @@ J2KImageProxy::prepare (optional<dcp::Size> target_size) const
        }
 
        _target_size = target_size;
+       _reduce = reduce;
+
+       return reduce;
 }
 
-shared_ptr<Image>
+pair<shared_ptr<Image>, int>
 J2KImageProxy::image (optional<dcp::NoteHandler>, optional<dcp::Size> target_size) const
 {
-       prepare (target_size);
+       int const reduce = prepare (target_size);
 
        shared_ptr<Image> image (new Image (_pixel_format, _decompressed->size(), true));
 
@@ -169,7 +176,7 @@ J2KImageProxy::image (optional<dcp::NoteHandler>, optional<dcp::Size> target_siz
                }
        }
 
-       return image;
+       return make_pair (image, reduce);
 }
 
 void

diff --git a/src/lib/j2k_image_proxy.h b/src/lib/j2k_image_proxy.h
index 3680de1114c857dcf80e5b64321499293f183107..77d53a9e58440f56c09e2b815d50a8c05cb37074 100644 (file)
--- a/src/lib/j2k_image_proxy.h
+++ b/src/lib/j2k_image_proxy.h
@@ -50,7 +50,7 @@ public:
 
        J2KImageProxy (boost::shared_ptr<cxml::Node> xml, boost::shared_ptr<Socket> socket);
 
-       boost::shared_ptr<Image> image (
+       std::pair<boost::shared_ptr<Image>, int> image (
                boost::optional<dcp::NoteHandler> note = boost::optional<dcp::NoteHandler> (),
                boost::optional<dcp::Size> size = boost::optional<dcp::Size> ()
                ) const;
@@ -59,7 +59,7 @@ public:
        void send_binary (boost::shared_ptr<Socket>) const;
        /** @return true if our image is definitely the same as another, false if it is probably not */
        bool same (boost::shared_ptr<const ImageProxy>) const;
-       void prepare (boost::optional<dcp::Size> = boost::optional<dcp::Size>()) const;
+       int prepare (boost::optional<dcp::Size> = boost::optional<dcp::Size>()) const;
        AVPixelFormat pixel_format () const {
                return _pixel_format;
        }
@@ -85,6 +85,7 @@ private:
        boost::optional<dcp::Eye> _eye;
        mutable boost::shared_ptr<dcp::OpenJPEGImage> _decompressed;
        mutable boost::optional<dcp::Size> _target_size;
+       mutable boost::optional<int> _reduce;
        AVPixelFormat _pixel_format;
        mutable boost::mutex _mutex;
        boost::optional<int> _forced_reduction;

diff --git a/src/lib/magick_image_proxy.cc b/src/lib/magick_image_proxy.cc
index 0fd4a5edb2bb36b355f4fb18996394c3e87266b0..b12a81db554e0ddbdb8ebe5d7532d2945bb2845a 100644 (file)
--- a/src/lib/magick_image_proxy.cc
+++ b/src/lib/magick_image_proxy.cc
@@ -32,6 +32,8 @@
 
 using std::string;
 using std::cout;
+using std::pair;
+using std::make_pair;
 using boost::shared_ptr;
 using boost::optional;
 using boost::dynamic_pointer_cast;
@@ -66,13 +68,13 @@ MagickImageProxy::MagickImageProxy (shared_ptr<cxml::Node>, shared_ptr<Socket> s
        delete[] data;
 }
 
-shared_ptr<Image>
+pair<shared_ptr<Image>, int>
 MagickImageProxy::image (optional<dcp::NoteHandler>, optional<dcp::Size>) const
 {
        boost::mutex::scoped_lock lm (_mutex);
 
        if (_image) {
-               return _image;
+               return make_pair (_image, 0);
        }
 
        Magick::Image* magick_image = 0;
@@ -137,7 +139,7 @@ MagickImageProxy::image (optional<dcp::NoteHandler>, optional<dcp::Size>) const
 
        delete magick_image;
 
-       return _image;
+       return make_pair (_image, 0);
 }
 
 void

diff --git a/src/lib/magick_image_proxy.h b/src/lib/magick_image_proxy.h
index 32d2f7efb1dd0e772b55ec5fcf15396197609ca3..7d2b69afeaf4aabec4b115354143cdd1d6414d98 100644 (file)
--- a/src/lib/magick_image_proxy.h
+++ b/src/lib/magick_image_proxy.h
@@ -29,7 +29,7 @@ public:
        explicit MagickImageProxy (boost::filesystem::path);
        MagickImageProxy (boost::shared_ptr<cxml::Node> xml, boost::shared_ptr<Socket> socket);
 
-       boost::shared_ptr<Image> image (
+       std::pair<boost::shared_ptr<Image>, int> image (
                boost::optional<dcp::NoteHandler> note = boost::optional<dcp::NoteHandler> (),
                boost::optional<dcp::Size> size = boost::optional<dcp::Size> ()
                ) const;

diff --git a/src/lib/player_video.cc b/src/lib/player_video.cc
index 8eb39efedca6484e47b50d7fe679cef8d317197e..a50b196a200152caf782a108c45eb1fdaddc5fbc 100644 (file)
--- a/src/lib/player_video.cc
+++ b/src/lib/player_video.cc
@@ -34,6 +34,7 @@ extern "C" {
 
 using std::string;
 using std::cout;
+using std::pair;
 using boost::shared_ptr;
 using boost::weak_ptr;
 using boost::dynamic_pointer_cast;
@@ -112,7 +113,9 @@ PlayerVideo::set_subtitle (PositionImage image)
 shared_ptr<Image>
 PlayerVideo::image (dcp::NoteHandler note, function<AVPixelFormat (AVPixelFormat)> pixel_format, bool aligned, bool fast) const
 {
-       shared_ptr<Image> im = _in->image (optional<dcp::NoteHandler> (note), _inter_size);
+       pair<shared_ptr<Image>, int> prox = _in->image (optional<dcp::NoteHandler> (note), _inter_size);
+       shared_ptr<Image> im = prox.first;
+       int const reduce = prox.second;
 
        Crop total_crop = _crop;
        switch (_part) {
@@ -132,6 +135,15 @@ PlayerVideo::image (dcp::NoteHandler note, function<AVPixelFormat (AVPixelFormat
                break;
        }
 
+       if (reduce > 0) {
+               /* Scale the crop down to account for the scaling that has already happened in ImageProxy::image */
+               int const r = pow(2, reduce);
+               total_crop.left /= r;
+               total_crop.right /= r;
+               total_crop.top /= r;
+               total_crop.bottom /= r;
+       }
+
        dcp::YUVToRGB yuv_to_rgb = dcp::YUV_TO_RGB_REC601;
        if (_colour_conversion) {
                yuv_to_rgb = _colour_conversion.get().yuv_to_rgb();

diff --git a/src/lib/raw_image_proxy.cc b/src/lib/raw_image_proxy.cc
index 094b50d058c2bef13cece2d1dbe8df402bce4e21..c3b565f3c768ade554b11d622ab6ea1e6896b304 100644 (file)
--- a/src/lib/raw_image_proxy.cc
+++ b/src/lib/raw_image_proxy.cc
@@ -1,5 +1,5 @@
 /*
-    Copyright (C) 2014 Carl Hetherington <cth@carlh.net>
+    Copyright (C) 2014-2018 Carl Hetherington <cth@carlh.net>
 
     This file is part of DCP-o-matic.
 
@@ -31,6 +31,8 @@ extern "C" {
 #include "i18n.h"
 
 using std::string;
+using std::pair;
+using std::make_pair;
 using boost::shared_ptr;
 using boost::dynamic_pointer_cast;
 using boost::optional;
@@ -52,10 +54,10 @@ RawImageProxy::RawImageProxy (shared_ptr<cxml::Node> xml, shared_ptr<Socket> soc
        _image->read_from_socket (socket);
 }
 
-shared_ptr<Image>
+pair<shared_ptr<Image>, int>
 RawImageProxy::image (optional<dcp::NoteHandler>, optional<dcp::Size>) const
 {
-       return _image;
+       return make_pair (_image, 0);
 }
 
 void
@@ -81,7 +83,7 @@ RawImageProxy::same (shared_ptr<const ImageProxy> other) const
                return false;
        }
 
-       return (*_image.get()) == (*rp->image().get());
+       return (*_image.get()) == (*rp->image().first.get());
 }
 
 AVPixelFormat

diff --git a/src/lib/raw_image_proxy.h b/src/lib/raw_image_proxy.h
index 9f8424eb34df8061154ee96925b679a8d6939890..78b7db0c763cf1e53bbfcb188fef025012a57850 100644 (file)
--- a/src/lib/raw_image_proxy.h
+++ b/src/lib/raw_image_proxy.h
@@ -29,7 +29,7 @@ public:
        explicit RawImageProxy (boost::shared_ptr<Image>);
        RawImageProxy (boost::shared_ptr<cxml::Node> xml, boost::shared_ptr<Socket> socket);
 
-       boost::shared_ptr<Image> image (
+       std::pair<boost::shared_ptr<Image>, int> image (
                boost::optional<dcp::NoteHandler> note = boost::optional<dcp::NoteHandler> (),
                boost::optional<dcp::Size> size = boost::optional<dcp::Size> ()
                ) const;

diff --git a/test/image_test.cc b/test/image_test.cc
index 7780c94cc4f428ef77e30729469bb7efe5724a1a..5adc3560330623f8e4f6e44245043a7c7d5a9d24 100644 (file)
--- a/test/image_test.cc
+++ b/test/image_test.cc
@@ -139,7 +139,7 @@ void
 alpha_blend_test_one (AVPixelFormat format, string suffix)
 {
        shared_ptr<MagickImageProxy> proxy (new MagickImageProxy (private_data / "prophet_frame.tiff"));
-       shared_ptr<Image> raw = proxy->image();
+       shared_ptr<Image> raw = proxy->image().first;
        shared_ptr<Image> background = raw->convert_pixel_format (dcp::YUV_TO_RGB_REC709, format, true, false);
 
        shared_ptr<Image> overlay (new Image (AV_PIX_FMT_BGRA, dcp::Size(431, 891), true));
@@ -261,7 +261,7 @@ BOOST_AUTO_TEST_CASE (merge_test2)
 BOOST_AUTO_TEST_CASE (crop_scale_window_test)
 {
        shared_ptr<MagickImageProxy> proxy(new MagickImageProxy("test/data/flat_red.png"));
-       shared_ptr<Image> raw = proxy->image();
+       shared_ptr<Image> raw = proxy->image().first;
        shared_ptr<Image> out = raw->crop_scale_window(Crop(), dcp::Size(1998, 836), dcp::Size(1998, 1080), dcp::YUV_TO_RGB_REC709, AV_PIX_FMT_YUV420P, true, false);
        shared_ptr<Image> save = out->scale(dcp::Size(1998, 1080), dcp::YUV_TO_RGB_REC709, AV_PIX_FMT_RGB24, false, false);
        write_image(save, "build/test/crop_scale_window_test.png", "RGB");