From 3720f66bb1ce9ffb7fe9c4e32893054f3f1749a8 Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Sat, 29 Oct 2022 17:39:45 -0500 Subject: [PATCH] LibVideo: Set CodingIndependentCodePoints in its member functions This moves the setting of code points in CICP structs to member functions completely so that the code having to set these code points can be much cleaner. --- Userland/Applications/VideoPlayer/main.cpp | 5 ++- .../Color/CodingIndependentCodePoints.h | 27 +++++++++++--- .../Libraries/LibVideo/MatroskaDocument.h | 37 +++++++++---------- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/Userland/Applications/VideoPlayer/main.cpp b/Userland/Applications/VideoPlayer/main.cpp index 5ebe2773976..03fecaa3d97 100644 --- a/Userland/Applications/VideoPlayer/main.cpp +++ b/Userland/Applications/VideoPlayer/main.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include "LibVideo/Color/CodingIndependentCodePoints.h" #include #include #include @@ -90,8 +91,8 @@ ErrorOr serenity_main(Main::Arguments arguments) auto frame = frame_result.release_value(); auto& cicp = frame->cicp(); - video_track.color_format.replace_code_points_if_specified(cicp); - cicp.default_code_points_if_unspecified(Video::ColorPrimaries::BT709, Video::TransferCharacteristics::BT709, Video::MatrixCoefficients::BT709); + cicp.adopt_specified_values(video_track.color_format.to_cicp()); + cicp.default_code_points_if_unspecified({ Video::ColorPrimaries::BT709, Video::TransferCharacteristics::BT709, Video::MatrixCoefficients::BT709, Video::ColorRange::Studio }); auto convert_result = frame->output_to_bitmap(image); if (convert_result.is_error()) { diff --git a/Userland/Libraries/LibVideo/Color/CodingIndependentCodePoints.h b/Userland/Libraries/LibVideo/Color/CodingIndependentCodePoints.h index e393406c91f..5d42ace0274 100644 --- a/Userland/Libraries/LibVideo/Color/CodingIndependentCodePoints.h +++ b/Userland/Libraries/LibVideo/Color/CodingIndependentCodePoints.h @@ -73,8 +73,9 @@ enum class MatrixCoefficients : u8 { }; enum class ColorRange : u8 { - Studio = 0, // Y range 16..235, UV range 16..240 - Full = 1, // 0..255 + Unspecified, + Studio, // Y range 16..235, UV range 16..240 + Full, // 0..255 }; // https://en.wikipedia.org/wiki/Coding-independent_code_points @@ -97,14 +98,28 @@ public: constexpr ColorRange color_range() const { return m_color_range; } constexpr void set_color_range(ColorRange value) { m_color_range = value; } - constexpr void default_code_points_if_unspecified(ColorPrimaries cp, TransferCharacteristics tc, MatrixCoefficients mc) + constexpr void default_code_points_if_unspecified(CodingIndependentCodePoints cicp) { if (color_primaries() == ColorPrimaries::Unspecified) - set_color_primaries(cp); + set_color_primaries(cicp.color_primaries()); if (transfer_characteristics() == TransferCharacteristics::Unspecified) - set_transfer_characteristics(tc); + set_transfer_characteristics(cicp.transfer_characteristics()); if (matrix_coefficients() == MatrixCoefficients::Unspecified) - set_matrix_coefficients(mc); + set_matrix_coefficients(cicp.matrix_coefficients()); + if (color_range() == ColorRange::Unspecified) + set_color_range(cicp.color_range()); + } + + constexpr void adopt_specified_values(CodingIndependentCodePoints cicp) + { + if (cicp.color_primaries() != ColorPrimaries::Unspecified) + set_color_primaries(cicp.color_primaries()); + if (cicp.transfer_characteristics() != TransferCharacteristics::Unspecified) + set_transfer_characteristics(cicp.transfer_characteristics()); + if (cicp.matrix_coefficients() != MatrixCoefficients::Unspecified) + set_matrix_coefficients(cicp.matrix_coefficients()); + if (cicp.color_range() != ColorRange::Unspecified) + set_color_range(cicp.color_range()); } private: diff --git a/Userland/Libraries/LibVideo/MatroskaDocument.h b/Userland/Libraries/LibVideo/MatroskaDocument.h index fbe7faf061a..daa25378f91 100644 --- a/Userland/Libraries/LibVideo/MatroskaDocument.h +++ b/Userland/Libraries/LibVideo/MatroskaDocument.h @@ -65,27 +65,26 @@ public: u64 bits_per_channel = 0; ColorRange range = ColorRange::Unspecified; - Video::ColorRange full_or_studio_range() const + CodingIndependentCodePoints to_cicp() const { - // FIXME: Figure out what UseCICP should do here. Matroska specification did not - // seem to explain in the 'colour' section. When this is fixed, change - // replace_code_points_if_specified to match. - VERIFY(range == ColorRange::Full || range == ColorRange::Broadcast); - if (range == ColorRange::Full) - return Video::ColorRange::Full; - return Video::ColorRange::Studio; - } + Video::ColorRange color_range; + switch (range) { + case ColorRange::Full: + color_range = Video::ColorRange::Full; + break; + case ColorRange::Broadcast: + color_range = Video::ColorRange::Studio; + break; + case ColorRange::Unspecified: + case ColorRange::UseCICP: + // FIXME: Figure out what UseCICP should do here. Matroska specification did not + // seem to explain in the 'colour' section. When this is fixed, change + // replace_code_points_if_specified to match. + color_range = Video::ColorRange::Unspecified; + break; + } - void replace_code_points_if_specified(CodingIndependentCodePoints& cicp) const - { - if (color_primaries != ColorPrimaries::Unspecified) - cicp.set_color_primaries(color_primaries); - if (transfer_characteristics != TransferCharacteristics::Unspecified) - cicp.set_transfer_characteristics(transfer_characteristics); - if (matrix_coefficients != MatrixCoefficients::Unspecified) - cicp.set_matrix_coefficients(matrix_coefficients); - if (range != ColorRange::Unspecified && range != ColorRange::UseCICP) - cicp.set_color_range(full_or_studio_range()); + return { color_primaries, transfer_characteristics, matrix_coefficients, color_range }; } };