libass icon indicating copy to clipboard operation
libass copied to clipboard

[WiP] Introduce LayoutResX/Y script headers

Open MrSmile opened this issue 3 years ago • 15 comments

This is my follow-up to the relevant IRC discussion: how I think resolution-independence should be achieved. Also I think this can be a good place for a more persistent discussion on the matter.

Main points of this ASS extension:

  • New script headers LayoutResX and LayoutResY with the same semantics as PlayResX and PlayResY.
  • Lone LayoutResX or LayoutResY is ignored. This is suboptimal, but introduction of new failure modes would be hard, especially outside libass. Alternatively we can derive missing dimension based on some fixed ratio (16×9?) similar to PlayResXY.
  • Correctly paired LayoutResX and LayoutResY override video size in all internal calculations.

MrSmile avatar Feb 19 '22 21:02 MrSmile

IMO we shouldn't land this until we're fully ready to guarantee future compatibility with scripts targeting current libass, but this seems reasonable at that point. We should gate it behind whatever the explicit "this script targets libass and not vsfilter" flag ends up being. Also, once we make that break, we should default LayoutRes to PlayRes (rather than storage res) for files with the flag; that ultimately means that these new headers would likely only be useful for older scripts designed with a res mismatch being updated for new releases (which is a reasonable enough goal).

rcombs avatar Feb 19 '22 21:02 rcombs

I thought that the idea was to implement this extension in currently alive VSFilter forks too.

Also, once we make that break, we should default LayoutRes to PlayRes

I've implemented backward-compatible variant of that: default PlayRes to LayoutRes instead. So that new scripts aimed for sufficiently recent libass (and hopefully, VSFilter too) can use only LayoutRes.

MrSmile avatar Feb 19 '22 21:02 MrSmile

Oh, if we get this implemented in vsfilters then that'd be great, yeah.

rcombs avatar Feb 19 '22 22:02 rcombs

I've implemented backward-compatible variant of that: default PlayRes to LayoutRes instead.

I continue to very disagree with this idea, as it means new files will be uncannily incompatible (render but at the wrong resolution) with older renderers even if they don’t use any new features.

astiob avatar Feb 19 '22 22:02 astiob

I've implemented backward-compatible variant of that: default PlayRes to LayoutRes instead.

I continue to very disagree with this idea, as it means

I think the same - disagree with this idea.

If you want to derive defaults, then PlayerResX/Y already have defaults, so this should remain the same for compatibility.

If none of LayoutResX/Y are specified then it behaves like before - it's storage size if given, or (I believe that's what libass does) PlayerResX/Y if storage size is unknown.

The only question is what to do if only one of LayoutResX/Y are given, where one of three things can happen:

  • The value is ignored.
  • It's an error and libass refuses to render anything.
  • It uses some fixed default value or fixed derivation from other values.

I think the third option is the best here, and the specific is to derive the value so that the ratio between Layer and Player remains the same on both X and Y asis. E.g. if PlayerResX=PX and PlayerResY=PY, and LayoutResX=LX and LayoutResY is missing, then it would be PY*LX/PX .

avih avatar Feb 19 '22 22:02 avih

The only question is what to do if only one of LayoutResX/Y are given […]

I think the [“It uses some fixed default value or fixed derivation from other values.”] option is the best here, and the specific is to derive the value so that the ratio between Layer and Player remains the same on both X and Y asis. E.g. if PlayerResX=PX and PlayerResY=PY, and LayoutResX=LX and LayoutResY is missing, then it would be PY*LX/PX.

I agree with the proposed patch here: the less magic, the better. If we don’t have both LayoutResX/Y explicitly specified, ignore them completely.

astiob avatar Feb 19 '22 23:02 astiob

Regarding what happens for unpaired or partially invalid LayoutRes{X,Y}, I think it should either be ignored or follow exactly the same logic as unpaired/invalid PlayRes{X,Y} to not add more inconsistent magic fallbacks.

TheOneric avatar Feb 19 '22 23:02 TheOneric

I continue to very disagree with this idea, as it means new files will be uncannily incompatible (render but at the wrong resolution) with older renderers even if they don’t use any new features.

Basically, I want the improved ASS format to provide the following:

  • Achieve resolution-independence with one set of sizes.
  • Provide a trivial way to adopt an older subs for a new video.

Some backward compatibility with old renderers is an additional bonus, but I don't want to give up my first goal for infinite support of such players. Especially when my proposal don't change much in the grand scheme of things: it's the subs provider that ultimately decides on such compatibility. My suggestion is merely changing compatibility condition from "LayoutRes = VideoRes" to "LayoutRes = VideoRes and PlayRes is present". If a group decides to drop compatibility, for example, shipping the same subs for different video resolutions, then they can immediately benefit from my proposal.

Also, I think that a hard break is better than subtle, as it'll be simpler to identify the cause and recommend to update the player.

I think it should either be ignored or follow exactly the same logic as unpaired/invalid PlayRes{X,Y} to not add more inconsistent magic fallbacks.

Logic for unpaired PlayRes is too complicated and outdated, so I've decided to ignore unpaired header.

MrSmile avatar Feb 20 '22 18:02 MrSmile

Also, I think that a hard break is better than subtle

However, this is not a hard break that you’re proposing. It’s an uncanny soft break: files still render and are mostly usable, but they look huge. A hard break is being proposed by rcombs for a new format: make new files display nothing at all in old renderers.

I also disagree that hard breaks are all that much better, but that’s not what I want to focus on for this particular PR.

My suggestion is merely changing compatibility condition from "LayoutRes = VideoRes" to "LayoutRes = VideoRes and PlayRes is present".

No, the current compatibility condition is far more complex than “LayoutRes = VideoRes”. If you don’t use blur or transforms, LayoutRes and VideoRes don’t matter at all. (And even if you do use blur but only in small amounts to make text “not look way too sharp”, it still doesn’t really matter in practice. Only transforms and big blur are really affected. I’m assuming ScaledBorderAndShadow, of course.) You’re proposing that we encourage new files that don’t use blur or transforms—which is most files—to become broken in old renderers while gaining nothing in exchange.

Regarding what happens for unpaired or partially invalid LayoutRes{X,Y}, I think it should either be ignored or follow exactly the same logic as unpaired/invalid PlayRes{X,Y} to not add more inconsistent magic fallbacks.

I agree (so I find the 16:9 proposal that was briefly mentioned especially unappealing). And of these, as mentioned, I prefer the ignore option, but I’d be fine with either.

astiob avatar Feb 20 '22 23:02 astiob

You’re proposing that we encourage new files that don’t use blur or transforms

No, I'm not proposing to encourage anyone, at least for now (instead, it's better to not advertise that feature to sub providers). If there're no blurs/transforms, then they can use only PlayRes as before, new format extension isn't for them anyway. I simply want to have a possibility to omit repetition for the far future, when old players gradually phase out. It's impossible to design a good format without some degree of foresight and only considering immediate needs.

MrSmile avatar Feb 21 '22 00:02 MrSmile

Also, I think that a hard break is better than subtle, as it'll be simpler to identify the cause and recommend to up date the player.

It can be identified by the presence of LayoutRes{X,Y} headers.


[MrSmile:] Logic for unpaired PlayRes is too complicated and outdated, so I've decided to ignore unpaired header.

[astiob:] And of these, as mentioned, I prefer the ignore option, but I’d be fine with either.

So it seems we all agree on ignoring unpaired LayoutRes{X,Y} as the best approach :)

TheOneric avatar Feb 21 '22 20:02 TheOneric

Regarding the arguments about compatibility and breaks here and on IRC:

The way I see it, since we also want to get VSFs on-board, adding LayoutRes or similar, is not intended to create a new improved format, but it’s a retroactive addition to the existing v4+ and v4++ format. So from this standpoint taking compatibility with old renders into account makes a lot of sense to me. Of course since it’s an addition we cannot achieve 100%-compatibility with old renderers, but we can try to limit breaks as much as possible. (Where this is less of a or depending on the scope no concern, is an actual new format revision)

Having LayoutRes and PlayRes as two independent value-pairs and telling sub-authoring software to always — without any possibility to manually override or ignore this — set LayoutRes to the actual storage res when one is available, is the best “least breakage” approach I can come up with.
The only concerns are with hand-written/modified subs which set LayoutRes to something other than the video’s storage resolution and reused subtitles where the new video has a different storage resolution, which also didn't work before.

Letting PlayRes default to LayoutRes on the other hand, makes breakages more likely by exposing another possibility of creating files incompatible with old renderers, without any benefit for rendering and reusing subs — only that hand-written subs can be two lines shorter. Since the intended benefit is for hand-written subs, we also can't hand-waive this with recommendations for and implementation-details in common authoring software. Regardless of what we recommend or discourage, if it’s possible to merge Playres and LayoutRes into just LayoutRes and we expect hand-written subs, it is pretty much guaranteed someone will do just that.
(On a new format revision we can merge the resolution headers though)

TheOneric avatar Feb 21 '22 20:02 TheOneric

I've found a problem with the code from this PR: subs from @TheOneric's anamorphic sample with corresponding LayoutRes headers looks wrong over non-anamorphic video. So par calculation/interpretation should be changed too.

Regardless of what we recommend or discourage, if it’s possible to merge Playres and LayoutRes into just LayoutRes and we expect hand-written subs, it is pretty much guaranteed someone will do just that.

And it would be perfectly normal if the first such event happens several years from now.

MrSmile avatar Feb 24 '22 01:02 MrSmile

Fixed par calculation.

MrSmile avatar Feb 26 '22 20:02 MrSmile

The only open question is whether LayoutRes* should affect PlayRes* fallbacks, which isn't a purely technical decision but involves personal preference. To figure out if and how this can proceed I asked on IRC a while ago and suggested even if there's no unanimous favourite we could just take a vote if nobody feels strongly enough about this to veto the not-preferred option. None of MrSmile, rcombs and myself would veto either outcome.

@astiob: can you state your current preference or other thoughts on how this can/should proceed?

TheOneric avatar Aug 03 '22 21:08 TheOneric

@astiob: can you state your current preference or other thoughts on how this can/should proceed?

Honestly, I’m more conflicted than ever, and unfortunately that feeling borders on apathy: “if I don’t know which is better, does the choice even matter?”

I don’t like the idea of half-breaking all existing ASS-processing software in new scripts such that the software recognizes ASS, parses and validates it successfully, but grossly misinterprets the resolution. And this isn’t only about end-user software renderers (which, let’s say, we’ll thoroughly patch now and make sure updated builds are easily available for install when the time comes): editors, batch processors, converters, scripts, hardware renderers, you name it. PlayResX/Y is a fundamental property of an ASS script, and you can do a lot of things with an ASS script without having to understand—much less render—all tags and fonts and whatnot, but if you get the basic resolution wrong, I imagine there’s more venue for catastrophic results. And the proposed change implies that once PlayRes headers start being omitted, they’ll be omitted even from the simplest scripts that don’t use any complex features and could be easily handled by even the dumbest and oldest processors, but the header change will preclude this.

On the other hand, I do see how it feels stupid having to specify both PlayRes and LayoutRes even if they’re the same value—which most of the time they will be. And in a new format when it ever comes to fruition, I certainly expect to see only one resolution, and it may as well be called LayoutRes. And at the end of the day, software can mostly be corrected to account for LayoutRes piece by piece when someone encounters a misinterpreted new file, even though this will cause some inconvenience.

Another downside that has just jumped into my mind is that if/when we start seeing most new files made with LayoutRes and without PlayRes, someone writing completely new software for ASS processing may end up unaware that PlayRes exists at all. Of course, that is also an issue that’s fixable once someone encounters it.

The question comes down to [inconvenience of having to fix all existing software and to abandon software that can’t be fixed] vs [inconvenience of having to duplicate resolution headers in new files]. I’m inclined to think that the first is worse, but I’m no longer sure.

By the way (part serious, part attempt at reductio ad absurdum): if we’re gonna change the headers to reduce their number, why not make the presence of valid LayoutRes switch the ScaledBorderAndShadow default to yes?

astiob avatar Aug 14 '22 13:08 astiob

Thanks for your in-depth response.

For me personally, your explanation reinforced my pre-existing opinion of not-affected fallback being less bad. The downsides of affected fallback all seem like a real pain, which will require work and awareness of the change for years to come. While the need for two more, perhaps seemingly redundant headers, doesn't strike me as much of a downside, since a) with the intent of teaching Aegisub about them, this will be handled automatically for most authors and b) if authors forget or don't want to add the new headers the situation won't be worse then now (but it’s trivial for those who obtained the subs to later add the new headers if desired).

Regardless of one’s preferred option:

[...] unfortunately that feeling borders on apathy: “if I don’t know which is better, does the choice even matter?”

I want to point out, being apathetic and not changing anything, also has its cost and I believe they are worse than either option. We already know, some (sloppy) sub authors are releasing multi-resolution muxes explicitly targeting MPC-HC ISR’s or mpv --sub-ass-vsfilter-blur-compat’s scaling behaviour to avoid the need for manually adjusting subs to each resolution. The demand for simpler resampling options isn't limited to this though. As mentioned on IRC before, there are xy-VSFilter forks which patched the scaling behaviour in an incompatible way and this incompatible patch even made it into the (I believe) more widely used pinterf/xy-VSFilter claiming to be a "fix" (https://github.com/pinterf/xy-VSFilter/issues/17). Fortunately, it didn't make it into a release yet. Once it does though, people might start to created incompatible subs unintentionally. There's a real chance, not doing anything, will result in all the different VSFilter projects receiving – possibly slightly different – scaling ""fixes"", leading to further fragmentation of ASS, which will be way worse than the two options discussed here. (I haven't contacted pinterf yet, as I'm hoping to settle this first and then be able to point to a clear path of how to better approach scaling without breaking compat with old and other current renderers.)

.

Currently, MrSmile strongly prefers affected fallback, I do the opposite. Tbh I'm not sure if @rcombs’ response on IRC was intended as neutrality or a (weak?) preference for unaffected fallback. If it’s the former and with your response also leading towards neutrality, we would have a stalemate and I'm not sure what to do.

Btw, which VSFilter maintainers do we all need to contanct and get an agreement with before we consider this widely accepted enough to add to libass? I imagine at least Cyberbeing, probably pinterf and while MPC-HC ISR’s behaviour without LayoutRes is already incompatible, we could also contact clsid2 and ideally reduce the incompatibilities when LayoutRes is set. Anyone else?

TheOneric avatar Aug 14 '22 17:08 TheOneric

I have a weak preference for the default on PlayRes remaining the same, but I'd rather we move forwards on this either way than do nothing (as long as we have vsfilter buy-in).

rcombs avatar Aug 14 '22 17:08 rcombs

Thanks for clarifying, I guess this overall makes for a weak bias in favour of unaffected fallback then. And from previous discussion on IRC explicitly and from astiob’s response here implicitly, it appears like indeed noone would block either option if this allows things to move forward. If I got this impression wrong, please complain!

Whose cooperation at a minimum is required for you to consider it “VSFilter buy-in”?

TheOneric avatar Aug 14 '22 17:08 TheOneric

Whatever's necessary for commonly-used builds of aegisub to have support, really (bonus points if common MPC builds get it as well). Not sure who that entails exactly.

rcombs avatar Aug 14 '22 17:08 rcombs

Unfortunately, I'm not sure either. Common Aegisub builds I'm aware of are:

  • old builds from Aegisub/Aegisub
  • Ristellise/Aegisub
  • wangqr/Aegisub
  • TypesettingTools/Aegisub
  • arch1t3cht/Aegisub

The first two aren't (currently) maintained and won't receive this (for now). I assume old Aegisub/Aegisub builds used Cyberbeing's xy-VSFilter as xy-VSFilter, but sometimes also bundled VSFilterMod. An older release news from Ristellise indicates Cyberbeing’s VSfilter being used too. The last three all bundle a xy-VSFilter as far as I can tell, but I do not know if it’s from Cyberbeing’s repo or a fork.

TheOneric avatar Aug 14 '22 18:08 TheOneric

I want to point out, being apathetic and not changing anything, also has its cost and I believe they are worse than either option.

Oh, absolutely. It was more “whatever, just merge something already” than “whatever, let’s just not do this”.

which VSFilter maintainers do we all need to contanct and get an agreement with before we consider this widely accepted enough to add to libass? I imagine at least Cyberbeing, probably pinterf and while MPC-HC ISR’s behaviour without LayoutRes is already incompatible, we could also contact clsid2 and ideally reduce the incompatibilities when LayoutRes is set.

Cyberbeing hasn’t been active on GitHub for two years and didn’t respond when I tried to contact him a year ago. I’d advise to try contacting him still but not to expect a response. Practically speaking, pinterf maintains the only active xy-VSFilter/XySubFilter fork and clsid2 maintains the only active MPC-HC fork, and they should be the first points of contact for any VSFilter changes. Pull requests with ready-to-apply patches would be ideal, as in my understanding neither of them is very well-versed in VSFilter internals as pertains to ASS business logic.

And yes, I’d recommend trying to get this into MPC-HC ISR, in the sense that if the new headers are set, all renderers should have the same behaviour, even if the variance in behaviour in the new headers’ absence stays.

As for Aegisub’s renderers, I believe it’s a matter of poking each build maintainer and telling them, “hey, new versions of xy-VSFilter and libass are out that have this great new feature, please update them both”. But besides that (after that), Aegisub itself should start writing the new headers.

astiob avatar Aug 14 '22 18:08 astiob

As for Aegisub’s renderers, I believe it’s a matter of poking each build maintainer and telling them, “hey, new versions of xy-VSFilter and libass are out that have this great new feature, please update them both”.

Oh, and to be clear, that would entail using pinterf’s xy-VSFilter, as the only current xy-VSFilter. My understanding is that its only current compatibility issue that we’re aware of is this resolution thing, which would by then be fixed along with the introduction of LayoutRes.

astiob avatar Aug 14 '22 19:08 astiob

My understanding is that its only current compatibility issue that we’re aware of is this resolution thing, which would by then be fixed along with the introduction of LayoutRes.

Going through the changes documented in pinterf’s README, most seem like they only affect the AViSynth filter, fix crashes or build/optimisation tweaks. However, there's also a change moving from 32bit timestamps to 64bit ones, see https://github.com/pinterf/xy-VSFilter/commit/09176fce2897c8cdc1edb00878c56cf029ab19cf and the preceding commit. In libass, the timestamps were changed to act like 32bit ones in 0.15.0, here: https://github.com/libass/libass/commit/7913e4a64c704a3b82719d70920b5b153b43d254 . Were there any real-world files known to rely on the old 32bit behaviour? Note though, that in case we want to match newer pinterf/xy-VSFilter, just reverting to full 64-bit timestamps still won't perfectly match pinterf, as it didn't just move to counting millisecond in 64bit, but for some reason counting 100ns increments in 64bit.

TheOneric avatar Aug 15 '22 14:08 TheOneric

Superseded by #641, to avoid pullling VSFilter maintainers into a discussion with backlog.

TheOneric avatar Aug 20 '22 23:08 TheOneric

I'll write here since this is only relevant to the backlog here:

The last three all bundle a xy-VSFilter as far as I can tell, but I do not know if it’s from Cyberbeing’s repo or a fork.

Currently, the TypesettingTools fork always bundles the latest version of pinterf's xy-VSFilter - this is implemented here. Only VSFilter.dll is shipped (see here for the installer and here for the portable version). By extension, the same is true for my fork. After a new release on pinterf's fork, all new clean builds on these repositories would use the updated version.

The version used in wangqr's fork doesn't seem to be documented - it's just pulled from wangqr's server here. However, its currently shipped VSFilter.dll is identical to the one in pinterf's latest release.

As for libass, the TypesettingTools fork dynamically links against whatever is installed or statically links the meson-pr branch of TypesettingTools/libass . Even if #330 is not merged by the time this is added, it would be a simple matter of rebasing that branch or pointing Aegisub to a rebased version.

arch1t3cht avatar Aug 25 '22 11:08 arch1t3cht