From ed98574ce24d1ba246b843731529c06f6a4dd78f Mon Sep 17 00:00:00 2001 From: Oneric Date: Thu, 3 Nov 2022 18:20:47 +0100 Subject: [PATCH 1/5] Make CRenderedTextSubtitle::Init private It is only used from the member function RenderEx(...) and being able to rely on it not being used from the outside simplifies the next commit. Deinit(..) has users outside the class, but is also not relevant for the planned change. --- src/subtitles/RTS.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/subtitles/RTS.h b/src/subtitles/RTS.h index 0beaf741..33e79524 100644 --- a/src/subtitles/RTS.h +++ b/src/subtitles/RTS.h @@ -457,9 +457,13 @@ class CRenderedTextSubtitle : public CSubPicProviderImpl, public ISubStream, pub virtual void Copy(CSimpleTextSubtitle& sts); virtual void Empty(); -public: +private: + /* + * Will call Deinit() + */ bool Init(const CRectCoor2& video_rect, const CRectCoor2& subtitle_target_rect, - const SIZE& original_video_size); // will call Deinit() + const SIZE& original_video_size); +public: void Deinit(); DECLARE_IUNKNOWN From 8a46df19666191f6fcaa44811123c34f165f358e Mon Sep 17 00:00:00 2001 From: Oneric Date: Thu, 3 Nov 2022 18:42:01 +0100 Subject: [PATCH 2/5] Introduce LayoutRes{X,Y} script headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rendering ASS depends on the video’s storage resolution for several tags. Thus transferring a subtitle file to a different version of a video, with e.g. higher resolution or anamorphic squeezing undone, requires adjusting all those tags. Affected are \be, \blur, \frx, \fry and if ScaledBorderAndShadow is not set to "yes", also all tags related to border and shadow. This locks all but simple subtitle files to a specific video storage resolution. If one wants to release several different resolution simultaneously, the same source is reencoded to undo anamorphic squeezing, or a new higher resolution source appears, it is required to manually adjust affected tags and each video version needs a different subtitle file. This is a pain point, and resulted in some releases just relying on user overrides of players, or incompatibly patched renderers to avoid the required manual adjustments. By adding new headers, which will replace the original video storage resolution in all calculations, authors will be able to create files which can be reused across different resolutions as long as the display aspect ratio stays the same. Hopefully this will also halt the spread of incomapatible patches and overrides, which just result in broken files and further ASS fragmentation. For simplicity and to avoid surprises, LayoutRes* headers only take effect if both are present and set to values larger than zero. If LayoutRes{X,Y} is set to corresponds to the actual storage resolution of the video the subs are authored and initially released with, at least the initial/main version will also be effectively compatible to older renderers, which do not understand the new headers yet. This will grant users some time to upgrade and minimise friction from this retroactive format addition. The header's introduction is coordinated with other renderers, so that they'll be widely available amongt still maintain ASS implementations. As a followup, integration into active Aegisub forks and other common editors will also be pursued. Regarding the implementation in xy-VSFilter/xySubFilter: To find out where the video storage resolution is used, I followed the "originalVideoSize" field from include/SubRenderIntf.h to RTS.cpp. The property is however, also used in a bunch of places outside of src/subtitles. As I have no knowledge of those areas, I didn't change anything there, but it seems unlikely - and prelimanry testing seems to agree - that those have any effect on ASS rendering. To make it easy to spot the actual functional changes, the old original_video_size name was kept for most parts. A follow-up patch will do cosmetic-only renaming. --- src/subtitles/RTS.cpp | 13 ++++++++++++- src/subtitles/RTS.h | 2 ++ src/subtitles/STS.cpp | 10 ++++++++++ src/subtitles/STS.h | 1 + 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/subtitles/RTS.cpp b/src/subtitles/RTS.cpp index c2e73902..23086fcd 100644 --- a/src/subtitles/RTS.cpp +++ b/src/subtitles/RTS.cpp @@ -1858,6 +1858,11 @@ bool CRenderedTextSubtitle::Init( const CRectCoor2& video_rect, const CRectCoor2 { XY_LOG_INFO(_T("")); Deinit(); + + // Since this is only called from RenderEx(..), which already overrides + // original_video_size with m_layout_size when appropiate, we do not + // need to override it again and can just use original_size here + m_video_rect = CRect(video_rect.left*MAX_SUB_PIXEL, video_rect.top*MAX_SUB_PIXEL, video_rect.right*MAX_SUB_PIXEL, @@ -3336,7 +3341,7 @@ STDMETHODIMP CRenderedTextSubtitle::RenderEx(SubPicDesc& spd, REFERENCE_TIME rt, STDMETHODIMP CRenderedTextSubtitle::RenderEx( IXySubRenderFrame**subRenderFrame, int spd_type, const RECT& video_rect, const RECT& subtitle_target_rect, - const SIZE& original_video_size, + const SIZE& actual_original_video_size, REFERENCE_TIME rt, double fps ) { TRACE_RENDERER_REQUEST("Begin RenderEx rt"< 0 && m_layout_size.cy > 0) { + original_video_size.cx = m_layout_size.cx; + original_video_size.cy = m_layout_size.cy; + } + XyColorSpace color_space = XY_CS_ARGB; switch(spd_type) { diff --git a/src/subtitles/RTS.h b/src/subtitles/RTS.h index 33e79524..66179285 100644 --- a/src/subtitles/RTS.h +++ b/src/subtitles/RTS.h @@ -459,6 +459,8 @@ class CRenderedTextSubtitle : public CSubPicProviderImpl, public ISubStream, pub private: /* + * Expects the original_video_size parameter to already be + * overridden by LayoutRes headers when applicable. * Will call Deinit() */ bool Init(const CRectCoor2& video_rect, const CRectCoor2& subtitle_target_rect, diff --git a/src/subtitles/STS.cpp b/src/subtitles/STS.cpp index 7b1c61b9..a1a4c9cb 100644 --- a/src/subtitles/STS.cpp +++ b/src/subtitles/STS.cpp @@ -2060,6 +2060,16 @@ if(sver <= 4) style->scrAlignment = (style->scrAlignment&4) ? ((style->scrAlig : ret.m_dstScreenSize.cy * 4 / 3; } } + else if(entry == L"layoutresx") + { + try {ret.m_layout_size.cx = GetInt(buff);} + catch(...) {ret.m_layout_size = CSize(0, 0); return(false);} + } + else if(entry == L"layoutresy") + { + try {ret.m_layout_size.cy = GetInt(buff);} + catch(...) {ret.m_layout_size = CSize(0, 0); return(false);} + } else if(entry == L"wrapstyle") { try {ret.m_defaultWrapStyle = GetInt(buff);} diff --git a/src/subtitles/STS.h b/src/subtitles/STS.h index b191b24c..eef103b2 100644 --- a/src/subtitles/STS.h +++ b/src/subtitles/STS.h @@ -180,6 +180,7 @@ class CSimpleTextSubtitle int m_defaultWrapStyle; int m_collisions; bool m_fScaledBAS; + CSize m_layout_size; CSTSStyleMap m_styles; From 4b536f0497a7dbce4e35b1b55275f9c5a6cb0fb5 Mon Sep 17 00:00:00 2001 From: Oneric Date: Thu, 3 Nov 2022 18:49:41 +0100 Subject: [PATCH 3/5] Cosmetic renaming of original_video_size to layout_size Since it can now be overriden by script headers, the old name became misleading. --- src/subtitles/RTS.cpp | 26 +++++++++++--------------- src/subtitles/RTS.h | 7 ++++--- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/subtitles/RTS.cpp b/src/subtitles/RTS.cpp index 23086fcd..9fae771f 100644 --- a/src/subtitles/RTS.cpp +++ b/src/subtitles/RTS.cpp @@ -1854,27 +1854,23 @@ void CRenderedTextSubtitle::OnChanged() bool CRenderedTextSubtitle::Init( const CRectCoor2& video_rect, const CRectCoor2& subtitle_target_rect, - const SIZE& original_video_size ) + const SIZE& layout_size ) { XY_LOG_INFO(_T("")); Deinit(); - // Since this is only called from RenderEx(..), which already overrides - // original_video_size with m_layout_size when appropiate, we do not - // need to override it again and can just use original_size here - m_video_rect = CRect(video_rect.left*MAX_SUB_PIXEL, video_rect.top*MAX_SUB_PIXEL, video_rect.right*MAX_SUB_PIXEL, video_rect.bottom*MAX_SUB_PIXEL); m_subtitle_target_rect = CRect(subtitle_target_rect.left*MAX_SUB_PIXEL, subtitle_target_rect.top*MAX_SUB_PIXEL, subtitle_target_rect.right*MAX_SUB_PIXEL, subtitle_target_rect.bottom*MAX_SUB_PIXEL); - m_size = CSize(original_video_size.cx*MAX_SUB_PIXEL, original_video_size.cy*MAX_SUB_PIXEL); + m_size = CSize(layout_size.cx*MAX_SUB_PIXEL, layout_size.cy*MAX_SUB_PIXEL); - ASSERT(original_video_size.cx!=0 && original_video_size.cy!=0); + ASSERT(layout_size.cx!=0 && layout_size.cy!=0); - m_target_scale_x = video_rect.Width() * 1.0 / original_video_size.cx; - m_target_scale_y = video_rect.Height() * 1.0 / original_video_size.cy; + m_target_scale_x = video_rect.Width() * 1.0 / layout_size.cx; + m_target_scale_y = video_rect.Height() * 1.0 / layout_size.cy; return(true); } @@ -3341,7 +3337,7 @@ STDMETHODIMP CRenderedTextSubtitle::RenderEx(SubPicDesc& spd, REFERENCE_TIME rt, STDMETHODIMP CRenderedTextSubtitle::RenderEx( IXySubRenderFrame**subRenderFrame, int spd_type, const RECT& video_rect, const RECT& subtitle_target_rect, - const SIZE& actual_original_video_size, + const SIZE& original_video_size, REFERENCE_TIME rt, double fps ) { TRACE_RENDERER_REQUEST("Begin RenderEx rt"< 0 && m_layout_size.cy > 0) { - original_video_size.cx = m_layout_size.cx; - original_video_size.cy = m_layout_size.cy; + layout_size.cx = m_layout_size.cx; + layout_size.cy = m_layout_size.cy; } XyColorSpace color_space = XY_CS_ARGB; @@ -3402,9 +3398,9 @@ STDMETHODIMP CRenderedTextSubtitle::RenderEx( IXySubRenderFrame**subRenderFrame, subtitle_target_rect.top*MAX_SUB_PIXEL, subtitle_target_rect.right*MAX_SUB_PIXEL, subtitle_target_rect.bottom*MAX_SUB_PIXEL) - || m_size != CSize(original_video_size.cx*MAX_SUB_PIXEL, original_video_size.cy*MAX_SUB_PIXEL) ) + || m_size != CSize(layout_size.cx*MAX_SUB_PIXEL, layout_size.cy*MAX_SUB_PIXEL) ) { - if (!Init(cvideo_rect, subtitle_target_rect, original_video_size)) + if (!Init(cvideo_rect, subtitle_target_rect, layout_size)) { XY_LOG_FATAL("Failed to Init."); return E_FAIL; diff --git a/src/subtitles/RTS.h b/src/subtitles/RTS.h index 66179285..eaa55fd5 100644 --- a/src/subtitles/RTS.h +++ b/src/subtitles/RTS.h @@ -459,12 +459,13 @@ class CRenderedTextSubtitle : public CSubPicProviderImpl, public ISubStream, pub private: /* - * Expects the original_video_size parameter to already be - * overridden by LayoutRes headers when applicable. + * The layout_size parameter is either derived from + * valid LayoutRes{X,Y} script headers (both must be valid!) + * or if those do not exist, equal to the video's storage resolution. * Will call Deinit() */ bool Init(const CRectCoor2& video_rect, const CRectCoor2& subtitle_target_rect, - const SIZE& original_video_size); + const SIZE& layout_size); public: void Deinit(); From ad68731db70a8f7ade408be8c30c0a59911e95bc Mon Sep 17 00:00:00 2001 From: Oneric Date: Sun, 21 Aug 2022 14:01:45 +0200 Subject: [PATCH 4/5] Fix build in GHA environment Without this the build cannot find yasm.exe. Adjustments copied from xy-VSFilter-with-libass found here: https://github.com/Masaiki/xy-VSFilter. It is not tested on a local VS setup, because I don't have one. --- src/thirdparty/VirtualDub/Kasumi/Kasumi.vcxproj | 14 +++++++++++++- src/thirdparty/VirtualDub/system/system.vcxproj | 14 +++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/thirdparty/VirtualDub/Kasumi/Kasumi.vcxproj b/src/thirdparty/VirtualDub/Kasumi/Kasumi.vcxproj index d667a791..1d85de5b 100644 --- a/src/thirdparty/VirtualDub/Kasumi/Kasumi.vcxproj +++ b/src/thirdparty/VirtualDub/Kasumi/Kasumi.vcxproj @@ -39,6 +39,18 @@ + + $(VCInstallDir);$(VC_ExecutablePath_x86);$(CommonExecutablePath) + + + $(VCInstallDir);$(VC_ExecutablePath_x86);$(CommonExecutablePath) + + + $(VCInstallDir);$(VC_ExecutablePath_x64);$(CommonExecutablePath) + + + $(VCInstallDir);$(VC_ExecutablePath_x64);$(CommonExecutablePath) + h;..\h;%(AdditionalIncludeDirectories) @@ -200,4 +212,4 @@ - \ No newline at end of file + diff --git a/src/thirdparty/VirtualDub/system/system.vcxproj b/src/thirdparty/VirtualDub/system/system.vcxproj index 3482ea17..e44acf16 100644 --- a/src/thirdparty/VirtualDub/system/system.vcxproj +++ b/src/thirdparty/VirtualDub/system/system.vcxproj @@ -39,6 +39,18 @@ + + $(VCInstallDir);$(VC_ExecutablePath_x86);$(CommonExecutablePath) + + + $(VCInstallDir);$(VC_ExecutablePath_x86);$(CommonExecutablePath) + + + $(VCInstallDir);$(VC_ExecutablePath_x64);$(CommonExecutablePath) + + + $(VCInstallDir);$(VC_ExecutablePath_x64);$(CommonExecutablePath) + ..\h;h;%(AdditionalIncludeDirectories) @@ -192,4 +204,4 @@ - \ No newline at end of file + From 06571e86c8a3f05417a9b1c68cd721e8b014bdd4 Mon Sep 17 00:00:00 2001 From: Oneric Date: Tue, 16 Aug 2022 23:53:30 +0200 Subject: [PATCH 5/5] Add GHA CI and publish nightly builds --- .github/workflows/build.yml | 91 +++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 00000000..3cd91bae --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,91 @@ +name: Build VSFilter + +on: + push: + branches: [ci, xy_sub_filter_rc*, vsfilter_rc*, master] + +jobs: + build: + runs-on: windows-2019 + strategy: + fail-fast: false + matrix: + include: + - msarch: x64 + vsfplat: x64 + namesuf: x86-64 + - arch: x86 + vsfplat: Win32 + namesuf: x86-32 + defaults: + run: + shell: 'bash' + + steps: + - name: Determine configuration + id: config + run: | + # base name of GHA artifact + name="VSFilter+SubFilter" + # space delimited list of filename stems to delete from artifact + delete="" + + # If it runs for one of the main XySubFilter or xy-VSFilter dev branches, + # only publish compatible binaries. If it's some other branch, it's probably + # a transient testing build, so keep everything. + # ref.: https://github.com/Cyberbeing/xy-VSFilter/pull/18#issuecomment-1302638201 + REF="${{ github.ref }}" + REF="${REF#refs/heads/}" + if [ "$REF" = "master" ] || [ "$REF" = "vsfilter_rc" ] ; then + name="VSFilter" + delete="XySubFilter" + # xy-VSFilter specific changes weren't merged back into the + # XySubFilter branch, so it normally shouldn't be used to build + # xy-VSFilter. But in pinterf/xy-VSFilter there’s only one branch. + #elif echo "$REF" | grep -qE '^xy_sub_filter_rc' ; then + # name="SubFilter" + # delete="VSFilter" + fi + + echo "name=$name" >> $GITHUB_OUTPUT + echo "delete=$delete" >> $GITHUB_OUTPUT + + - name: checkout code + uses: actions/checkout@v3 + with: + # We need full history to allow finding the tag in build prep + fetch-depth: 0 + + - name: install yasm + shell: cmd + run: | + git clone --depth=1 https://github.com/ShiftMediaProject/VSYASM.git + .\VSYASM\install_script.bat + + # VS2019 is 16.x; VS2022 17.x + - name: Setup VS2019 + uses: microsoft/setup-msbuild@v1.1 + with: + msbuild-architecture: matrix.msarch + # vs-version: '[16.01,16.11]' + + - name: Build xy-VSFilter and/or XySubFilter + run: | + export VS160COMNTOOLS="C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/Common7/Tools/" + bash ./build_vsfilter.sh -platform "${{ matrix.vsfplat }}" -compiler VS2019 + + - name: Prune incompatible files + if: steps.config.outputs.delete != '' + run: | + for dir in bin/lib*/*/Release ; do + for bn in ${{ steps.config.outputs.delete }} ; do + rm -f "$dir"/"$bn".* + done + done + + - name: Publish output + uses: actions/upload-artifact@v3 + with: + name: xy-${{ steps.config.outputs.name }}_nightly_${{ matrix.namesuf }} + path: | + bin/lib*/*/Release