From 73e1dc1b60cd5c2076e897e6f07b8ea47b77a46b Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 17 Oct 2013 07:23:45 -0700 Subject: [PATCH] Honor the color matrix tag from the subtitles file when appropriate If the script's matrix matches the video's matrix, use that even if Force BT.601 is on, and if it's TV.601, use that even if Force BT.601 is off. This should mostly eliminate the common problems resulting from mixed values of the Force BT.601 option. Closes #1649. --- aegisub/src/factory_manager.h | 22 ++++++- aegisub/src/frame_main.cpp | 25 ++++---- aegisub/src/threaded_frame_source.cpp | 4 +- aegisub/src/threaded_frame_source.h | 2 +- aegisub/src/video_context.cpp | 2 +- aegisub/src/video_provider_avs.cpp | 6 +- aegisub/src/video_provider_avs.h | 2 +- aegisub/src/video_provider_dummy.cpp | 2 +- aegisub/src/video_provider_dummy.h | 2 +- aegisub/src/video_provider_ffmpegsource.cpp | 65 +++++++++++---------- aegisub/src/video_provider_ffmpegsource.h | 4 +- aegisub/src/video_provider_manager.cpp | 4 +- aegisub/src/video_provider_manager.h | 4 +- aegisub/src/video_provider_yuv4mpeg.cpp | 2 +- aegisub/src/video_provider_yuv4mpeg.h | 2 +- 15 files changed, 86 insertions(+), 62 deletions(-) diff --git a/aegisub/src/factory_manager.h b/aegisub/src/factory_manager.h index cc80e3b76..a1134d60d 100644 --- a/aegisub/src/factory_manager.h +++ b/aegisub/src/factory_manager.h @@ -68,8 +68,24 @@ public: } }; -template -class Factory : public FactoryBase { +template +class Factory : public FactoryBase { + typedef Base *(*func)(Arg1, Arg2); + +public: + static std::unique_ptr Create(std::string const& name, Arg1 a1, Arg2 a2) { + auto factory = FactoryBase::Find(name); + return factory ? std::unique_ptr(factory(a1, a2)) : nullptr; + } + + template + static void Register(std::string name, bool hide = false, std::vector subTypes = std::vector()) { + FactoryBase::DoRegister([](Arg1 a1, Arg2 a2) -> Base * { return new T(a1, a2); }, name, hide, subTypes); + } +}; + +template +class Factory : public FactoryBase { typedef Base *(*func)(Arg1); public: @@ -85,7 +101,7 @@ public: }; template -class Factory : public FactoryBase { +class Factory : public FactoryBase { typedef Base *(*func)(); public: diff --git a/aegisub/src/frame_main.cpp b/aegisub/src/frame_main.cpp index 0976a89e4..eae81dc02 100644 --- a/aegisub/src/frame_main.cpp +++ b/aegisub/src/frame_main.cpp @@ -582,6 +582,7 @@ void FrameMain::OnAudioClose() { void FrameMain::OnSubtitlesOpen() { UpdateTitle(); + auto vc = context->videoController; /// @todo figure out how to move this to the relevant controllers without /// prompting for each file loaded/unloaded @@ -592,9 +593,9 @@ void FrameMain::OnSubtitlesOpen() { auto keyframes = config::path->MakeAbsolute(context->ass->GetScriptInfo("Keyframes File"), "?script"); auto audio = config::path->MakeAbsolute(context->ass->GetScriptInfo("Audio URI"), "?script"); - bool videoChanged = !blockVideoLoad && video != context->videoController->GetVideoName(); - bool timecodesChanged = vfr != context->videoController->GetTimecodesName(); - bool keyframesChanged = keyframes != context->videoController->GetKeyFramesName(); + bool videoChanged = !blockVideoLoad && video != vc->GetVideoName(); + bool timecodesChanged = vfr != vc->GetTimecodesName(); + bool keyframesChanged = keyframes != vc->GetKeyFramesName(); bool audioChanged = !blockAudioLoad && audio != context->audioController->GetAudioURL(); // Check if there is anything to change @@ -607,6 +608,8 @@ void FrameMain::OnSubtitlesOpen() { if (autoLoadMode == 2) { if (wxMessageBox(_("Do you want to load/unload the associated files?"), _("(Un)Load files?"), wxYES_NO | wxCENTRE, this) != wxYES) { SetDisplayMode(1, 1); + if (vc->IsLoaded() && vc->GetProvider()->GetColorSpace() != context->ass->GetScriptInfo("YCbCr Matrix")) + vc->Reload(); return; } } @@ -616,20 +619,20 @@ void FrameMain::OnSubtitlesOpen() { // Video if (videoChanged) { - context->videoController->SetVideo(video); - if (context->videoController->IsLoaded()) { - context->videoController->JumpToFrame(context->ass->GetUIStateAsInt("Video Position")); + vc->SetVideo(video); + if (vc->IsLoaded()) { + vc->JumpToFrame(context->ass->GetUIStateAsInt("Video Position")); std::string arString = context->ass->GetUIState("Video Aspect Ratio"); if (boost::starts_with(arString, "c")) { double ar = 0.; agi::util::try_parse(arString.substr(1), &ar); - context->videoController->SetAspectRatio(ar); + vc->SetAspectRatio(ar); } else { int ar = 0; if (agi::util::try_parse(arString, &ar) && ar >= 0 && ar < 4) - context->videoController->SetAspectRatio((AspectRatio)ar); + vc->SetAspectRatio((AspectRatio)ar); } double videoZoom = 0.; @@ -637,9 +640,11 @@ void FrameMain::OnSubtitlesOpen() { context->videoDisplay->SetZoom(videoZoom); } } + else if (vc->IsLoaded() && vc->GetProvider()->GetColorSpace() != context->ass->GetScriptInfo("YCbCr Matrix")) + vc->Reload(); - context->videoController->LoadTimecodes(vfr); - context->videoController->LoadKeyframes(keyframes); + vc->LoadTimecodes(vfr); + vc->LoadKeyframes(keyframes); // Audio if (audioChanged) { diff --git a/aegisub/src/threaded_frame_source.cpp b/aegisub/src/threaded_frame_source.cpp index 10d3740e3..aaa888159 100644 --- a/aegisub/src/threaded_frame_source.cpp +++ b/aegisub/src/threaded_frame_source.cpp @@ -109,10 +109,10 @@ static std::unique_ptr get_subs_provider(wxEvtHandler *parent } } -ThreadedFrameSource::ThreadedFrameSource(agi::fs::path const& video_filename, wxEvtHandler *parent) +ThreadedFrameSource::ThreadedFrameSource(agi::fs::path const& video_filename, std::string const& colormatrix, wxEvtHandler *parent) : worker(agi::dispatch::Create()) , subs_provider(get_subs_provider(parent)) -, video_provider(VideoProviderFactory::GetProvider(video_filename)) +, video_provider(VideoProviderFactory::GetProvider(video_filename, colormatrix)) , parent(parent) , frame_number(-1) , time(-1.) diff --git a/aegisub/src/threaded_frame_source.h b/aegisub/src/threaded_frame_source.h index 5f1fc2d57..9f3688582 100644 --- a/aegisub/src/threaded_frame_source.h +++ b/aegisub/src/threaded_frame_source.h @@ -106,7 +106,7 @@ public: /// @brief Constructor /// @param videoFileName File to open /// @param parent Event handler to send FrameReady events to - ThreadedFrameSource(agi::fs::path const& filename, wxEvtHandler *parent); + ThreadedFrameSource(agi::fs::path const& filename, std::string const& colormatrix, wxEvtHandler *parent); ~ThreadedFrameSource(); }; diff --git a/aegisub/src/video_context.cpp b/aegisub/src/video_context.cpp index ff3e8896f..e6d0d7b46 100644 --- a/aegisub/src/video_context.cpp +++ b/aegisub/src/video_context.cpp @@ -129,7 +129,7 @@ void VideoContext::SetVideo(const agi::fs::path &filename) { bool commit_subs = false; try { - provider.reset(new ThreadedFrameSource(filename, this)); + provider.reset(new ThreadedFrameSource(filename, context->ass->GetScriptInfo("YCbCr Matrix"), this)); video_provider = provider->GetVideoProvider(); video_filename = filename; diff --git a/aegisub/src/video_provider_avs.cpp b/aegisub/src/video_provider_avs.cpp index 58e32f215..8c2777102 100644 --- a/aegisub/src/video_provider_avs.cpp +++ b/aegisub/src/video_provider_avs.cpp @@ -53,7 +53,7 @@ #include #endif -AvisynthVideoProvider::AvisynthVideoProvider(agi::fs::path const& filename) +AvisynthVideoProvider::AvisynthVideoProvider(agi::fs::path const& filename, std::string const& colormatrix) { agi::acs::CheckFileRead(filename); @@ -146,7 +146,9 @@ file_exit: if (!vi.IsRGB()) { /// @todo maybe read ColorMatrix hints for d2v files? AVSValue args[2] = { script, "Rec601" }; - if (!OPT_GET("Video/Force BT.601")->GetBool() && (vi.width > 1024 || vi.height >= 600)) { + bool force_bt601 = OPT_GET("Video/Force BT.601")->GetBool() || colormatrix == "TV.601"; + bool bt709 = vi.width > 1024 || vi.height >= 600; + if (bt709 && (!force_bt601 || colormatrix == "TV.709")) { args[1] = "Rec709"; colorspace = "TV.709"; } diff --git a/aegisub/src/video_provider_avs.h b/aegisub/src/video_provider_avs.h index 27b64c451..93942e338 100644 --- a/aegisub/src/video_provider_avs.h +++ b/aegisub/src/video_provider_avs.h @@ -54,7 +54,7 @@ class AvisynthVideoProvider: public VideoProvider { AVSValue Open(agi::fs::path const& filename); public: - AvisynthVideoProvider(agi::fs::path const& filename); + AvisynthVideoProvider(agi::fs::path const& filename, std::string const& colormatrix); std::shared_ptr GetFrame(int n); diff --git a/aegisub/src/video_provider_dummy.cpp b/aegisub/src/video_provider_dummy.cpp index bdbeaba6b..609242d35 100644 --- a/aegisub/src/video_provider_dummy.cpp +++ b/aegisub/src/video_provider_dummy.cpp @@ -84,7 +84,7 @@ void DummyVideoProvider::Create(double fps, int frames, int width, int height, u } } -DummyVideoProvider::DummyVideoProvider(agi::fs::path const& filename) { +DummyVideoProvider::DummyVideoProvider(agi::fs::path const& filename, std::string const&) { if (!boost::starts_with(filename.string(), "?dummy")) throw agi::fs::FileNotFound(std::string("Attempted creating dummy video provider with non-dummy filename")); diff --git a/aegisub/src/video_provider_dummy.h b/aegisub/src/video_provider_dummy.h index c2ae5defe..d29658d2c 100644 --- a/aegisub/src/video_provider_dummy.h +++ b/aegisub/src/video_provider_dummy.h @@ -63,7 +63,7 @@ class DummyVideoProvider : public VideoProvider { public: /// Create a dummy video from a string returned from MakeFilename - DummyVideoProvider(agi::fs::path const& filename); + DummyVideoProvider(agi::fs::path const& filename, std::string const& colormatix); /// Create a dummy video from separate parameters /// @param fps Frame rate of the dummy video diff --git a/aegisub/src/video_provider_ffmpegsource.cpp b/aegisub/src/video_provider_ffmpegsource.cpp index 9e54ebf6f..01bb68998 100644 --- a/aegisub/src/video_provider_ffmpegsource.cpp +++ b/aegisub/src/video_provider_ffmpegsource.cpp @@ -48,7 +48,31 @@ #include #include -FFmpegSourceVideoProvider::FFmpegSourceVideoProvider(agi::fs::path const& filename) try +namespace { +std::string colormatrix_description(int cs, int cr) { + // Assuming TV for unspecified + std::string str = cr == FFMS_CR_JPEG ? "PC" : "TV"; + + switch (cs) { + case FFMS_CS_RGB: + return "None"; + break; + case FFMS_CS_BT709: + return str + ".709"; + case FFMS_CS_FCC: + return str + ".FCC"; + case FFMS_CS_BT470BG: + case FFMS_CS_SMPTE170M: + return str + ".601"; + case FFMS_CS_SMPTE240M: + return str + ".240M"; + default: + throw VideoOpenError("Unknown video color space"); + } +} +} + +FFmpegSourceVideoProvider::FFmpegSourceVideoProvider(agi::fs::path const& filename, std::string const& colormatrix) try : VideoSource(nullptr, FFMS_DestroyVideoSource) , VideoInfo(nullptr) , Width(-1) @@ -61,7 +85,7 @@ FFmpegSourceVideoProvider::FFmpegSourceVideoProvider(agi::fs::path const& filena SetLogLevel(); - LoadVideo(filename); + LoadVideo(filename, colormatrix); } catch (std::string const& err) { throw VideoOpenError(err); @@ -70,7 +94,7 @@ catch (const char * err) { throw VideoOpenError(err); } -void FFmpegSourceVideoProvider::LoadVideo(agi::fs::path const& filename) { +void FFmpegSourceVideoProvider::LoadVideo(agi::fs::path const& filename, std::string const& colormatrix) { FFMS_Indexer *Indexer = FFMS_CreateIndexer(filename.string().c_str(), &ErrInfo); if (!Indexer) { if (ErrInfo.SubType == FFMS_ERROR_FILE_READ) @@ -168,44 +192,21 @@ void FFmpegSourceVideoProvider::LoadVideo(agi::fs::path const& filename) { else DAR = double(Width) / Height; - // Assuming TV for unspecified - ColorSpace = TempFrame->ColorRange == FFMS_CR_JPEG ? "PC" : "TV"; + auto CS = TempFrame->ColorSpace; + if (CS == FFMS_CS_UNSPECIFIED) + CS = Width > 1024 || Height >= 600 ? FFMS_CS_BT709 : FFMS_CS_BT470BG; + ColorSpace = colormatrix_description(CS, TempFrame->ColorRange); - int CS = TempFrame->ColorSpace; #if FFMS_VERSION >= ((2 << 24) | (17 << 16) | (1 << 8) | 0) - if (CS != FFMS_CS_RGB && CS != FFMS_CS_BT470BG && OPT_GET("Video/Force BT.601")->GetBool()) { + if (CS != FFMS_CS_RGB && CS != FFMS_CS_BT470BG && CS != colormatrix && (colormatrix == "TV.601" || OPT_GET("Video/Force BT.601")->GetBool())) { if (FFMS_SetInputFormatV(VideoSource, FFMS_CS_BT470BG, TempFrame->ColorRange, FFMS_GetPixFmt(""), &ErrInfo)) throw VideoOpenError(std::string("Failed to set input format: ") + ErrInfo.Buffer); CS = FFMS_CS_BT470BG; + ColorSpace = colormatrix_description(CS, TempFrame->ColorRange); } #endif - switch (CS) { - case FFMS_CS_RGB: - ColorSpace = "None"; - break; - case FFMS_CS_BT709: - ColorSpace += ".709"; - break; - case FFMS_CS_UNSPECIFIED: - ColorSpace += Width > 1024 || Height >= 600 ? "709" : "601"; - break; - case FFMS_CS_FCC: - ColorSpace += ".FCC"; - break; - case FFMS_CS_BT470BG: - case FFMS_CS_SMPTE170M: - ColorSpace += ".601"; - break; - case FFMS_CS_SMPTE240M: - ColorSpace += ".240M"; - break; - default: - throw VideoOpenError("Unknown video color space"); - break; - } - const int TargetFormat[] = { FFMS_GetPixFmt("bgra"), -1 }; if (FFMS_SetOutputFormatV2(VideoSource, TargetFormat, Width, Height, FFMS_RESIZER_BICUBIC, &ErrInfo)) { throw VideoOpenError(std::string("Failed to set output format: ") + ErrInfo.Buffer); diff --git a/aegisub/src/video_provider_ffmpegsource.h b/aegisub/src/video_provider_ffmpegsource.h index 96988d784..9222a5e1a 100644 --- a/aegisub/src/video_provider_ffmpegsource.h +++ b/aegisub/src/video_provider_ffmpegsource.h @@ -53,10 +53,10 @@ class FFmpegSourceVideoProvider : public VideoProvider, FFmpegSourceProvider { char FFMSErrMsg[1024]; ///< FFMS error message FFMS_ErrorInfo ErrInfo; ///< FFMS error codes/messages - void LoadVideo(agi::fs::path const& filename); + void LoadVideo(agi::fs::path const& filename, std::string const& colormatrix); public: - FFmpegSourceVideoProvider(agi::fs::path const& filename); + FFmpegSourceVideoProvider(agi::fs::path const& filename, std::string const& colormatrix); std::shared_ptr GetFrame(int n); diff --git a/aegisub/src/video_provider_manager.cpp b/aegisub/src/video_provider_manager.cpp index 8e6e122d9..116feac81 100644 --- a/aegisub/src/video_provider_manager.cpp +++ b/aegisub/src/video_provider_manager.cpp @@ -47,7 +47,7 @@ #include #include -std::unique_ptr VideoProviderFactory::GetProvider(agi::fs::path const& video_file) { +std::unique_ptr VideoProviderFactory::GetProvider(agi::fs::path const& video_file, std::string const& colormatrix) { std::vector factories = GetClasses(OPT_GET("Video/Provider")->GetString()); factories.insert(factories.begin(), "YUV4MPEG"); factories.insert(factories.begin(), "Dummy"); @@ -60,7 +60,7 @@ std::unique_ptr VideoProviderFactory::GetProvider(agi::fs::path c for (auto const& factory : factories) { std::string err; try { - auto provider = Create(factory, video_file); + auto provider = Create(factory, video_file, colormatrix); LOG_I("manager/video/provider") << factory << ": opened " << video_file; return provider->WantsCaching() ? agi::util::make_unique(std::move(provider)) : std::move(provider); } diff --git a/aegisub/src/video_provider_manager.h b/aegisub/src/video_provider_manager.h index 5b022818a..595dac07c 100644 --- a/aegisub/src/video_provider_manager.h +++ b/aegisub/src/video_provider_manager.h @@ -19,8 +19,8 @@ #include -class VideoProviderFactory : public Factory { +class VideoProviderFactory : public Factory { public: - static std::unique_ptr GetProvider(agi::fs::path const& video_file); + static std::unique_ptr GetProvider(agi::fs::path const& video_file, std::string const& colormatrix); static void RegisterProviders(); }; diff --git a/aegisub/src/video_provider_yuv4mpeg.cpp b/aegisub/src/video_provider_yuv4mpeg.cpp index 3ba023585..eec44a6d8 100644 --- a/aegisub/src/video_provider_yuv4mpeg.cpp +++ b/aegisub/src/video_provider_yuv4mpeg.cpp @@ -58,7 +58,7 @@ /// @brief Constructor /// @param filename The filename to open -YUV4MPEGVideoProvider::YUV4MPEGVideoProvider(agi::fs::path const& filename) +YUV4MPEGVideoProvider::YUV4MPEGVideoProvider(agi::fs::path const& filename, std::string const&) : sf(nullptr) , inited(false) , w (0) diff --git a/aegisub/src/video_provider_yuv4mpeg.h b/aegisub/src/video_provider_yuv4mpeg.h index 5d8432686..efd3413cb 100644 --- a/aegisub/src/video_provider_yuv4mpeg.h +++ b/aegisub/src/video_provider_yuv4mpeg.h @@ -129,7 +129,7 @@ class YUV4MPEGVideoProvider : public VideoProvider { int IndexFile(); public: - YUV4MPEGVideoProvider(agi::fs::path const& filename); + YUV4MPEGVideoProvider(agi::fs::path const& filename, std::string const&); ~YUV4MPEGVideoProvider(); std::shared_ptr GetFrame(int n);