From dec761de1fb40066bd74ffe4a7c74edc10e59bec Mon Sep 17 00:00:00 2001 From: arch1t3cht Date: Thu, 24 Oct 2024 20:20:32 +0200 Subject: [PATCH 1/2] Fix, simplify, and outsource ffms2 color space override logic Actually support all YCbCr Matrix values instead of only a 601 matrix. Simplify the override logic and move most of it outside of the ffms2 provider so it can be shared with other providers. Also stop erroring out on unknown video color spaces. This is not yet perfect (the guessing logic could be improved, invalid matrices should probably default to TV.601, and the added functions feel a bit out of place in video_provider_manager.cpp and are partially redundant with ycbcr_conv.cpp) but it's a lot better and more maintainable than before. --- src/include/aegisub/video_provider.h | 39 +++++++++++++ src/video_provider_ffmpegsource.cpp | 84 +++++++--------------------- src/video_provider_manager.cpp | 69 +++++++++++++++++++++++ 3 files changed, 129 insertions(+), 63 deletions(-) diff --git a/src/include/aegisub/video_provider.h b/src/include/aegisub/video_provider.h index 43ff245459..ac878cc9a4 100644 --- a/src/include/aegisub/video_provider.h +++ b/src/include/aegisub/video_provider.h @@ -41,6 +41,45 @@ struct VideoFrame; +/// Color matrix constants matching the constants in ffmpeg +/// (specifically libavutil's AVColorSpace) and/or H.273. +typedef enum AGI_ColorSpaces { + AGI_CS_RGB = 0, + AGI_CS_BT709 = 1, + AGI_CS_UNSPECIFIED = 2, + AGI_CS_FCC = 4, + AGI_CS_BT470BG = 5, + AGI_CS_SMPTE170M = 6, + AGI_CS_SMPTE240M = 7, + AGI_CS_YCOCG = 8, + AGI_CS_BT2020_NCL = 9, + AGI_CS_BT2020_CL = 10, + AGI_CS_SMPTE2085 = 11, + AGI_CS_CHROMATICITY_DERIVED_NCL = 12, + AGI_CS_CHROMATICITY_DERIVED_CL = 13, + AGI_CS_ICTCP = 14 +} AGI_ColorSpaces; + +/// Color matrix constants matching the constants in ffmpeg +/// (specifically libavutil's AVColorRange) and/or H.273. +typedef enum AGI_ColorRanges { + AGI_CR_UNSPECIFIED = 0, + AGI_CR_MPEG = 1, // 219*2^(n-8), i.e. 16-235 with 8-bit samples + AGI_CR_JPEG = 2 // 2^n-1, or "fullrange" +} AGI_ColorRanges; + +namespace ColorMatrix { + +std::string colormatrix_description(int CS, int CR); + +std::pair parse_colormatrix(std::string matrix); + +void guess_colorspace(int &CS, int &CR, int Width, int Height); + +void override_colormatrix(int &CS, int &CR, std::string matrix, int Width, int Height); + +} + class VideoProvider { public: virtual ~VideoProvider() = default; diff --git a/src/video_provider_ffmpegsource.cpp b/src/video_provider_ffmpegsource.cpp index de3304576f..35507b6e2e 100644 --- a/src/video_provider_ffmpegsource.cpp +++ b/src/video_provider_ffmpegsource.cpp @@ -44,23 +44,6 @@ #include namespace { -typedef enum AGI_ColorSpaces { - AGI_CS_RGB = 0, - AGI_CS_BT709 = 1, - AGI_CS_UNSPECIFIED = 2, - AGI_CS_FCC = 4, - AGI_CS_BT470BG = 5, - AGI_CS_SMPTE170M = 6, - AGI_CS_SMPTE240M = 7, - AGI_CS_YCOCG = 8, - AGI_CS_BT2020_NCL = 9, - AGI_CS_BT2020_CL = 10, - AGI_CS_SMPTE2085 = 11, - AGI_CS_CHROMATICITY_DERIVED_NCL = 12, - AGI_CS_CHROMATICITY_DERIVED_CL = 13, - AGI_CS_ICTCP = 14 -} AGI_ColorSpaces; - /// @class FFmpegSourceVideoProvider /// @brief Implements video loading through the FFMS library. class FFmpegSourceVideoProvider final : public VideoProvider, FFmpegSourceProvider { @@ -70,13 +53,12 @@ class FFmpegSourceVideoProvider final : public VideoProvider, FFmpegSourceProvid int Width = -1; ///< width in pixels int Height = -1; ///< height in pixels - int CS = -1; ///< Reported colorspace of first frame - int CR = -1; ///< Reported colorrange of first frame + int VideoCS = -1; ///< Reported colorspace of first frame (or guessed if unspecified) + int VideoCR = -1; ///< Reported colorrange of first frame (or guessed if unspecified) double DAR; ///< display aspect ratio std::vector KeyFramesList; ///< list of keyframes agi::vfr::Framerate Timecodes; ///< vfr object std::string ColorSpace; ///< Colorspace name - std::string RealColorSpace; ///< Colorspace name char FFMSErrMsg[1024]; ///< FFMS error message FFMS_ErrorInfo ErrInfo; ///< FFMS error codes/messages @@ -91,12 +73,14 @@ class FFmpegSourceVideoProvider final : public VideoProvider, FFmpegSourceProvid void SetColorSpace(std::string const& matrix) override { if (matrix == ColorSpace) return; - if (matrix == RealColorSpace) - FFMS_SetInputFormatV(VideoSource, CS, CR, FFMS_GetPixFmt(""), nullptr); - else if (matrix == "TV.601") - FFMS_SetInputFormatV(VideoSource, AGI_CS_BT470BG, CR, FFMS_GetPixFmt(""), nullptr); - else - return; + + int CS = VideoCS; + int CR = VideoCR; + ColorMatrix::override_colormatrix(CS, CR, matrix, Width, Height); + + if (FFMS_SetInputFormatV(VideoSource, CS, CR, FFMS_GetPixFmt(""), &ErrInfo)) + throw VideoOpenError(std::string("Failed to set input format: ") + ErrInfo.Buffer); + ColorSpace = matrix; } @@ -114,34 +98,19 @@ class FFmpegSourceVideoProvider final : public VideoProvider, FFmpegSourceProvid agi::vfr::Framerate GetFPS() const override { return Timecodes; } std::string GetColorSpace() const override { return ColorSpace; } - std::string GetRealColorSpace() const override { return RealColorSpace; } + std::string GetRealColorSpace() const override { + std::string result = ColorMatrix::colormatrix_description(VideoCS, VideoCR); + if (result == "") { + return "None"; + } + return result; + } std::vector GetKeyFrames() const override { return KeyFramesList; }; std::string GetDecoderName() const override { return "FFmpegSource"; } bool WantsCaching() const override { return true; } bool HasAudio() const override { return has_audio; } }; -std::string colormatrix_description(int cs, int cr) { - // Assuming TV for unspecified - std::string str = cr == FFMS_CR_JPEG ? "PC" : "TV"; - - switch (cs) { - case AGI_CS_RGB: - return "None"; - case AGI_CS_BT709: - return str + ".709"; - case AGI_CS_FCC: - return str + ".FCC"; - case AGI_CS_BT470BG: - case AGI_CS_SMPTE170M: - return str + ".601"; - case AGI_CS_SMPTE240M: - return str + ".240M"; - default: - throw VideoOpenError("Unknown video color space"); - } -} - FFmpegSourceVideoProvider::FFmpegSourceVideoProvider(agi::fs::path const& filename, std::string const& colormatrix, agi::BackgroundRunner *br) try : FFmpegSourceProvider(br) , VideoSource(nullptr, FFMS_DestroyVideoSource) @@ -260,22 +229,11 @@ void FFmpegSourceVideoProvider::LoadVideo(agi::fs::path const& filename, std::st else DAR = double(Width) / Height; - int VideoCS = CS = TempFrame->ColorSpace; - CR = TempFrame->ColorRange; - - if (CS == AGI_CS_UNSPECIFIED) - CS = Width > 1024 || Height >= 600 ? AGI_CS_BT709 : AGI_CS_BT470BG; - RealColorSpace = ColorSpace = colormatrix_description(CS, CR); + VideoCS = TempFrame->ColorSpace; + VideoCR = TempFrame->ColorRange; + ColorMatrix::guess_colorspace(VideoCS, VideoCR, Width, Height); - if (CS != AGI_CS_RGB && CS != AGI_CS_BT470BG && ColorSpace != colormatrix && colormatrix == "TV.601") { - CS = AGI_CS_BT470BG; - ColorSpace = colormatrix_description(AGI_CS_BT470BG, CR); - } - - if (CS != VideoCS) { - if (FFMS_SetInputFormatV(VideoSource, CS, CR, FFMS_GetPixFmt(""), &ErrInfo)) - throw VideoOpenError(std::string("Failed to set input format: ") + ErrInfo.Buffer); - } + SetColorSpace(colormatrix); const int TargetFormat[] = { FFMS_GetPixFmt("bgra"), -1 }; if (FFMS_SetOutputFormatV2(VideoSource, TargetFormat, Width, Height, FFMS_RESIZER_BICUBIC, &ErrInfo)) diff --git a/src/video_provider_manager.cpp b/src/video_provider_manager.cpp index 8ec7c18fc6..b61dfcfba4 100644 --- a/src/video_provider_manager.cpp +++ b/src/video_provider_manager.cpp @@ -22,6 +22,7 @@ #include #include +#include #include @@ -33,6 +34,74 @@ std::unique_ptr CreateBSVideoProvider(agi::fs::path const&, std:: std::unique_ptr CreateCacheVideoProvider(std::unique_ptr); +namespace ColorMatrix { + +std::string colormatrix_description(int cs, int cr) { + // Assuming TV for unspecified + std::string str = cr == AGI_CR_JPEG ? "PC" : "TV"; + + switch (cs) { + case AGI_CS_RGB: + return "None"; + case AGI_CS_BT709: + return str + ".709"; + case AGI_CS_FCC: + return str + ".FCC"; + case AGI_CS_BT470BG: + case AGI_CS_SMPTE170M: + return str + ".601"; + case AGI_CS_SMPTE240M: + return str + ".240M"; + default: + return ""; + } +} + +std::pair parse_colormatrix(std::string matrix) { + int cs = AGI_CS_UNSPECIFIED; + int cr = AGI_CR_UNSPECIFIED; + + std::vector parts; + agi::Split(parts, matrix, '.'); + if (parts.size() == 2) { + if (parts[0] == "TV") { + cr = AGI_CR_MPEG; + } else if (parts[0] == "PC") { + cr = AGI_CR_JPEG; + } + + if (parts[1] == "709") { + cs = AGI_CS_BT709; + } else if (parts[1] == "601") { + cs = AGI_CS_BT470BG; + } else if (parts[1] == "FCC") { + cs = AGI_CS_FCC; + } else if (parts[1] == "240M") { + cs = AGI_CS_SMPTE240M; + } + } + + return std::make_pair(cs, cr); +} + +void guess_colorspace(int &CS, int &CR, int Width, int Height) { + if (CS == AGI_CS_UNSPECIFIED) + CS = Width > 1024 || Height >= 600 ? AGI_CS_BT709 : AGI_CS_BT470BG; + if (CR != AGI_CR_MPEG) + CR = AGI_CR_MPEG; +} + +void override_colormatrix(int &CS, int &CR, std::string matrix, int Width, int Height) { + guess_colorspace(CS, CR, Width, Height); + auto [oCS, oCR] = parse_colormatrix(matrix); + if (oCS != AGI_CS_UNSPECIFIED && oCR != AGI_CR_UNSPECIFIED) { + CS = oCS; + CR = oCR; + } +} + +} + namespace { struct factory { const char *name; From 1060e0591e199f39481bb90518066bee0e81d45e Mon Sep 17 00:00:00 2001 From: arch1t3cht Date: Fri, 25 Oct 2024 16:32:01 +0200 Subject: [PATCH 2/2] bestsource: Respect YCbCr Matrix header --- src/video_provider_bestsource.cpp | 48 +++++++++++++------------------ 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/src/video_provider_bestsource.cpp b/src/video_provider_bestsource.cpp index 5af37c4b81..d640d70082 100644 --- a/src/video_provider_bestsource.cpp +++ b/src/video_provider_bestsource.cpp @@ -61,6 +61,8 @@ class BSVideoProvider final : public VideoProvider { agi::vfr::Framerate Timecodes; AVPixelFormat pixfmt; std::string colorspace; + int video_cs = -1; // Reported or guessed color matrix of first frame + int video_cr = -1; // Reported or guessed color range of first frame bool has_audio = false; bool is_linear = false; @@ -72,7 +74,7 @@ class BSVideoProvider final : public VideoProvider { void GetFrame(int n, VideoFrame &out) override; - void SetColorSpace(std::string const& matrix) override { } // TODO Follow Aegisub's colorspace forcing? + void SetColorSpace(std::string const& matrix) override { colorspace = matrix; } int GetFrameCount() const override { return properties.NumFrames; }; @@ -82,33 +84,19 @@ class BSVideoProvider final : public VideoProvider { agi::vfr::Framerate GetFPS() const override { return Timecodes; }; std::string GetColorSpace() const override { return colorspace; }; - std::string GetRealColorSpace() const override { return colorspace; }; + std::string GetRealColorSpace() const override { + std::string result = ColorMatrix::colormatrix_description(video_cs, video_cr); + if (result == "") { + return "None"; + } + return result; + }; std::vector GetKeyFrames() const override { return Keyframes; }; std::string GetDecoderName() const override { return "BestSource"; }; bool WantsCaching() const override { return false; }; bool HasAudio() const override { return has_audio; }; }; -// Match the logic from the ffms2 provider, but directly use libavutil's constants and don't abort when encountering an unknown color space -std::string colormatrix_description(const AVFrame *frame) { - // Assuming TV for unspecified - std::string str = frame->color_range == AVCOL_RANGE_JPEG ? "PC" : "TV"; - - switch (frame->colorspace) { - case AVCOL_SPC_BT709: - return str + ".709"; - case AVCOL_SPC_FCC: - return str + ".FCC"; - case AVCOL_SPC_BT470BG: - case AVCOL_SPC_SMPTE170M: - return str + ".601"; - case AVCOL_SPC_SMPTE240M: - return str + ".240M"; - default: - return "None"; - } -} - BSVideoProvider::BSVideoProvider(agi::fs::path const& filename, std::string const& colormatrix, agi::BackgroundRunner *br) try : apply_rff(OPT_GET("Provider/Video/BestSource/Apply RFF")) , sws_context(nullptr, sws_freeContext) @@ -177,7 +165,9 @@ BSVideoProvider::BSVideoProvider(agi::fs::path const& filename, std::string cons // Decode the first frame to get the color space and pixel format std::unique_ptr frame(bs->GetFrame(0)); auto avframe = frame->GetAVFrame(); - colorspace = colormatrix_description(avframe); + video_cs = avframe->colorspace; + video_cr = avframe->color_range; + ColorMatrix::guess_colorspace(video_cs, video_cr, properties.Width, properties.Height); pixfmt = (AVPixelFormat) avframe->format; sws_context = sws_getContext( @@ -189,6 +179,7 @@ BSVideoProvider::BSVideoProvider(agi::fs::path const& filename, std::string cons throw VideoDecodeError("Cannot convert frame to RGB!"); } + SetColorSpace(colormatrix); } catch (BestSourceException const& err) { throw VideoOpenError(agi::format("Failed to create BestVideoSource: %s", + err.what())); @@ -210,16 +201,17 @@ void BSVideoProvider::GetFrame(int n, VideoFrame &out) { const AVFrame *frame = bsframe->GetAVFrame(); - int range = frame->color_range == AVCOL_RANGE_JPEG; - const int *coefficients = sws_getCoefficients(frame->colorspace == AVCOL_SPC_UNSPECIFIED ? AVCOL_SPC_BT709 : frame->colorspace); + int cs = frame->colorspace; + int cr = frame->color_range; + ColorMatrix::override_colormatrix(cs, cr, colorspace, properties.Width, properties.Height); + const int *coefficients = sws_getCoefficients(cs); if (frame->format != pixfmt || frame->width != properties.Width || frame->height != properties.Height) throw VideoDecodeError("Video has variable format!"); - // TODO apply color space forcing. sws_setColorspaceDetails(sws_context, - coefficients, range, - coefficients, range, + coefficients, cr == AVCOL_RANGE_JPEG, + coefficients, cr == AVCOL_RANGE_JPEG, 0, 1 << 16, 1 << 16); out.data.resize(frame->width * frame->height * 4);