diff options
| author | Carl Hetherington <cth@carlh.net> | 2025-07-22 00:48:42 +0200 |
|---|---|---|
| committer | Carl Hetherington <cth@carlh.net> | 2025-08-12 17:16:07 +0200 |
| commit | 98a8023dd774fd82c144a68039e0ea3131fb9142 (patch) | |
| tree | f14a3fb3f6a095f36b88c2c0e1066002cfb7132a | |
| parent | 9b73c143ce568bd8694e3a50f2fefc1ee3a03515 (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-x | run/tests | 2 | ||||
| -rw-r--r-- | src/lib/dcp_video.cc | 24 | ||||
| -rw-r--r-- | test/3042_regression_test.cc | 12 |
3 files changed, 35 insertions, 3 deletions
@@ -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); +} + |
