Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASS Format: Introduce LayoutRes{X,Y} script headers #39

Merged
merged 5 commits into from
Jan 18, 2023

Conversation

TheOneric
Copy link
Contributor

@TheOneric TheOneric commented Jan 5, 2023

Backport of Cyberbeing/xy-VSFilter#18 (see also: Cyberbeing/xy-VSFilter#19); the header was also introduced in libass and released as part of libass 0.17.0 about a month ago and clsid2 also works on implementing this in MPC-HC’s ISR.
In a sense also a follow up to #34.

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.
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.
Since it can now be overriden by script headers,
the old name became misleading.
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.
@kedaitinh12
Copy link

Hi, can you share binary??

@TheOneric
Copy link
Contributor Author

can you share binary??

xy-VSFilter builds may have subtle bugs, see #38. So if you want xy-VSFilter with LayoutRes* it might be better to use the nightly Cyberbeing/xy-VSFilter build from here.
For a XySubFilter build based on pinterf/xy-VSFilter see here.
I won't rerun the build when the artefacts expire; it’s fairly easy to run the GHA job oneself by just pushing to a ci branch on the own fork repo.

@kedaitinh12
Copy link

kedaitinh12 commented Jan 6, 2023

But can't pull request layout res to my fork. I want creat my binary myself
Screenshot_20230106_092412

@NBruderman
Copy link

@TheOneric Do you plan to make a fork with pinterf updates, but with the updated vsfilters patches that are missing here? (And with the fixes in this pull request too)

@TheOneric
Copy link
Contributor Author

can't pull request layout res to my fork.

YOu don't need pull requests for pushing to your own repo. You only need to enable GitHub Actions if it isn’t already (click the “Actions” tab) and directly push the relevant commits to a suitable branch like ci.

@NBruderman: Do you plan to make a fork with pinterf updates, but with the updated vsfilters patches that are missing here?

If the question is whether I’ll create a TheOneric/xy-VSFilter which goes beyond a private working copy for PRs and publish binaries there, then no, I won't do such a thing.

It hasn't even been two days yet since I opened this PR, so let’s just wait until @pinterf finds some time to look at and merge this.

@pinterf pinterf merged commit 15390e5 into pinterf:xy_sub_filter_rc5 Jan 18, 2023
@TheOneric TheOneric deleted the pinterf_layoutres branch January 18, 2023 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants