Fix colour range property for subsampled sources (#2357).

diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc
index 4bc88e1e4937ca2e896101ae8903c0539893cd7e..496ad5f1a2772574c1681becbe7e158d74151d8d 100644 (file)
--- a/src/lib/ffmpeg_content.cc
+++ b/src/lib/ffmpeg_content.cc
@@ -538,10 +538,12 @@ FFmpegContent::add_properties (shared_ptr<const Film> film, list<UserProperty>&
                video->add_properties (p);
 
                if (_bits_per_pixel) {
-                       /* Assuming there's three components, so bits per pixel component is _bits_per_pixel / 3 */
-                       int const lim_start = pow(2, _bits_per_pixel.get() / 3 - 4);
-                       int const lim_end = 235 * pow(2, _bits_per_pixel.get() / 3 - 8);
-                       int const total = pow(2, _bits_per_pixel.get() / 3);
+                       auto pixel_quanta_product = video->pixel_quanta().x * video->pixel_quanta().y;
+                       auto bits_per_main_pixel = pixel_quanta_product * _bits_per_pixel.get() / (pixel_quanta_product + 2);
+
+                       int const lim_start = pow(2, bits_per_main_pixel - 4);
+                       int const lim_end = 235 * pow(2, bits_per_main_pixel - 8);
+                       int const total = pow(2, bits_per_main_pixel);
 
                        switch (_color_range.get_value_or(AVCOL_RANGE_UNSPECIFIED)) {
                        case AVCOL_RANGE_UNSPECIFIED:
@@ -554,14 +556,14 @@ FFmpegContent::add_properties (shared_ptr<const Film> film, list<UserProperty>&
                                /// file is limited, so that not all possible values are valid.
                                p.push_back (
                                        UserProperty (
-                                               UserProperty::VIDEO, _("Colour range"), String::compose(_("Limited (%1-%2)"), lim_start, lim_end)
+                                               UserProperty::VIDEO, _("Colour range"), String::compose(_("Limited / video (%1-%2)"), lim_start, lim_end)
                                                )
                                        );
                                break;
                        case AVCOL_RANGE_JPEG:
                                /// TRANSLATORS: this means that the range of pixel values used in this
                                /// file is full, so that all possible pixel values are valid.
-                               p.push_back (UserProperty (UserProperty::VIDEO, _("Colour range"), String::compose (_("Full (0-%1)"), total)));
+                               p.push_back(UserProperty(UserProperty::VIDEO, _("Colour range"), String::compose(_("Full (0-%1)"), total - 1)));
                                break;
                        default:
                                DCPOMATIC_ASSERT (false);

diff --git a/test/data b/test/data
index f2aeabb8ac65f4a4e16199fcf2f07b6573521869..7c898df6d76579756af2f3e2577546764e97c139 160000 (submodule)
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit f2aeabb8ac65f4a4e16199fcf2f07b6573521869
+Subproject commit 7c898df6d76579756af2f3e2577546764e97c139

diff --git a/test/ffmpeg_properties_test.cc b/test/ffmpeg_properties_test.cc
new file mode 100644 (file)
index 0000000..15773ec
--- /dev/null
+++ b/test/ffmpeg_properties_test.cc
@@ -0,0 +1,65 @@
+/*
+    Copyright (C) 2022 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+
+#include "lib/compose.hpp"
+#include "lib/content.h"
+#include "lib/content_factory.h"
+#include "lib/user_property.h"
+#include "test.h"
+#include <boost/test/unit_test.hpp>
+
+
+using std::list;
+using std::string;
+
+
+static
+void
+colour_range_test(string name, boost::filesystem::path file, string ref)
+{
+       auto content = content_factory(file);
+       BOOST_REQUIRE(!content.empty());
+       auto film = new_test_film2(String::compose("ffmpeg_properties_test_%1", name), { content.front() });
+
+       auto properties = content.front()->user_properties(film);
+       auto iter = std::find_if(properties.begin(), properties.end(), [](UserProperty const& property) { return property.key == "Colour range"; });
+       BOOST_REQUIRE(iter != properties.end());
+       BOOST_CHECK_EQUAL(iter->value, ref);
+}
+
+
+BOOST_AUTO_TEST_CASE(ffmpeg_properties_test)
+{
+       colour_range_test("1", "test/data/test.mp4", "Unspecified");
+       colour_range_test("2", TestPaths::private_data() / "arrietty_JP-EN.mkv", "Limited / video (16-235)");
+       colour_range_test("3", "test/data/8bit_full_420.mp4", "Full (0-255)");
+       colour_range_test("4", "test/data/8bit_full_422.mp4", "Full (0-255)");
+       colour_range_test("5", "test/data/8bit_full_444.mp4", "Full (0-255)");
+       colour_range_test("6", "test/data/8bit_video_420.mp4", "Limited / video (16-235)");
+       colour_range_test("7", "test/data/8bit_video_422.mp4", "Limited / video (16-235)");
+       colour_range_test("8", "test/data/8bit_video_444.mp4", "Limited / video (16-235)");
+       colour_range_test("9", "test/data/10bit_full_420.mp4", "Full (0-1023)");
+       colour_range_test("10", "test/data/10bit_full_422.mp4", "Full (0-1023)");
+       colour_range_test("11", "test/data/10bit_full_444.mp4", "Full (0-1023)");
+       colour_range_test("12", "test/data/10bit_video_420.mp4", "Limited / video (64-940)");
+       colour_range_test("13", "test/data/10bit_video_422.mp4", "Limited / video (64-940)");
+       colour_range_test("14", "test/data/10bit_video_444.mp4", "Limited / video (64-940)");
+}

diff --git a/test/wscript b/test/wscript
index 9cf13dea5084be43eff8f1927ec5242abc43306b..fc63aac1eba2ce6d29580ba3a8656e7d2e56dcf0 100644 (file)
--- a/test/wscript
+++ b/test/wscript
@@ -85,6 +85,7 @@ def build(bld):
                  ffmpeg_decoder_sequential_test.cc
                  ffmpeg_encoder_test.cc
                  ffmpeg_examiner_test.cc
+                 ffmpeg_properties_test.cc
                  ffmpeg_pts_offset_test.cc
                  file_group_test.cc
                  file_log_test.cc