summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCarl Hetherington <cth@carlh.net>2025-07-22 00:48:42 +0200
committerCarl Hetherington <cth@carlh.net>2025-08-12 17:16:07 +0200
commit98a8023dd774fd82c144a68039e0ea3131fb9142 (patch)
treef14a3fb3f6a095f36b88c2c0e1066002cfb7132a
parent9b73c143ce568bd8694e3a50f2fefc1ee3a03515 (diff)
New/improved pixel format decision when converting to XYZ.
For a long time we would keep XYZ12LE, if that's what we have, otherwise ask FFmpeg to switch to RGB48LE. Then in 1d5c211dadb9a9dc2318adce86ca9c31b367cabe I tried to fix the case of an XYZ source mis-tagged as YUV. I changed things so that with no colour conversion we'd always ask FFmpeg to convert to XYZ. This meant that RGB sources with no colour conversion would get treatment by FFmpeg due to the RGB -> XYZ switch. Here we're going back to the more-or-less the "long time" behaviour when there is a conversion (keep XYZ12LE but otherwise convert to RGB48). When there's no conversion, keep RGB (to avoid the FFmpeg conversion from RGB -> XYZ) but convert everything else to XYZ.
-rwxr-xr-xrun/tests2
-rw-r--r--src/lib/dcp_video.cc24
-rw-r--r--test/3042_regression_test.cc12
3 files changed, 35 insertions, 3 deletions
diff --git a/run/tests b/run/tests
index c32565d73..18be05718 100755
--- a/run/tests
+++ b/run/tests
@@ -3,7 +3,7 @@
# e.g. --run_tests=foo
set -e
-PRIVATE_GIT="9c5ab14355ba4abb93d855ca1796bf80c4e1b153"
+PRIVATE_GIT="ddd3a3c138912b4588536c3b0ced7ef816facd12"
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
source $DIR/environment
diff --git a/src/lib/dcp_video.cc b/src/lib/dcp_video.cc
index 823325c3e..fdbd5774f 100644
--- a/src/lib/dcp_video.cc
+++ b/src/lib/dcp_video.cc
@@ -49,6 +49,9 @@
#include <dcp/warnings.h>
LIBDCP_DISABLE_WARNINGS
#include <libxml++/libxml++.h>
+extern "C" {
+#include <libavutil/pixdesc.h>
+}
LIBDCP_ENABLE_WARNINGS
#include <fmt/format.h>
#include <boost/asio.hpp>
@@ -102,7 +105,11 @@ DCPVideo::convert_to_xyz(shared_ptr<const PlayerVideo> frame)
shared_ptr<dcp::OpenJPEGImage> xyz;
if (frame->colour_conversion()) {
- auto image = frame->image(force(AV_PIX_FMT_RGB48LE), VideoRange::FULL, false);
+ auto conversion = [](AVPixelFormat fmt) {
+ return fmt == AV_PIX_FMT_XYZ12LE ? AV_PIX_FMT_XYZ12LE : AV_PIX_FMT_RGB48LE;
+ };
+
+ auto image = frame->image(conversion, VideoRange::FULL, false);
xyz = dcp::rgb_to_xyz(
image->data()[0],
image->size(),
@@ -110,7 +117,20 @@ DCPVideo::convert_to_xyz(shared_ptr<const PlayerVideo> frame)
frame->colour_conversion().get()
);
} else {
- auto image = frame->image(force(AV_PIX_FMT_RGB48LE), VideoRange::FULL, false);
+ auto conversion = [](AVPixelFormat fmt) {
+ auto const descriptor = av_pix_fmt_desc_get(fmt);
+ if (!descriptor) {
+ return fmt;
+ }
+
+ if (descriptor->flags & AV_PIX_FMT_FLAG_RGB) {
+ return AV_PIX_FMT_RGB48LE;
+ }
+
+ return AV_PIX_FMT_XYZ12LE;
+ };
+
+ auto image = frame->image(conversion, VideoRange::FULL, false);
xyz = make_shared<dcp::OpenJPEGImage>(image->data()[0], image->size(), image->stride()[0]);
}
diff --git a/test/3042_regression_test.cc b/test/3042_regression_test.cc
index 000fb6001..3660a3590 100644
--- a/test/3042_regression_test.cc
+++ b/test/3042_regression_test.cc
@@ -37,3 +37,15 @@ BOOST_AUTO_TEST_CASE(encode_xyz_from_prores_test)
check_one_frame(film->dir(film->dcp_name()), 0, TestPaths::private_data() / "dcp-o-matic_test_20250521_p3d65_frame0.j2c", 18);
}
+
+BOOST_AUTO_TEST_CASE(encode_xyz_from_dpx_test)
+{
+ auto content = content_factory(TestPaths::private_data() / "count.dpx")[0];
+ auto film = new_test_film("encode_xyz_from_dpx_test", { content });
+ content->video->unset_colour_conversion();
+
+ make_and_verify_dcp(film);
+
+ check_one_frame(film->dir(film->dcp_name()), 0, TestPaths::private_data() / "count.j2c", 18);
+}
+