From: Carl Hetherington Date: Fri, 21 Oct 2022 10:55:44 +0000 (+0200) Subject: Fix colour range property for subsampled sources (#2357). X-Git-Tag: v2.16.32~11 X-Git-Url: https://git.carlh.net/gitweb/?p=dcpomatic.git;a=commitdiff_plain;h=a9d99f7a77e8ab905464540c22f427dc83a3187c Fix colour range property for subsampled sources (#2357). --- diff --git a/src/lib/ffmpeg_content.cc b/src/lib/ffmpeg_content.cc index 4bc88e1e4..496ad5f1a 100644 --- a/src/lib/ffmpeg_content.cc +++ b/src/lib/ffmpeg_content.cc @@ -538,10 +538,12 @@ FFmpegContent::add_properties (shared_ptr film, list& 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 film, list& /// 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 f2aeabb8a..7c898df6d 160000 --- 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 index 000000000..15773ec2f --- /dev/null +++ b/test/ffmpeg_properties_test.cc @@ -0,0 +1,65 @@ +/* + Copyright (C) 2022 Carl Hetherington + + 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 . + +*/ + + +#include "lib/compose.hpp" +#include "lib/content.h" +#include "lib/content_factory.h" +#include "lib/user_property.h" +#include "test.h" +#include + + +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 9cf13dea5..fc63aac1e 100644 --- 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