Repository navigation
Use ass_set_storage_size to compensate for target SAR - #19
JustinTArthur wants to merge 2 commits into
Conversation
Avoid when LayoutRes comes from subtitles file.
| } | ||
|
|
||
| /* A track with LayoutRes headers of its own is laid out for what they say | ||
| and nothing here reaches it, so not even a frame of the clip is needed. */ |
There was a problem hiding this comment.
If you are blending a preexisting .ass file (as opposed to generating one on the fly as in core.sub.Subtitle), you must not do any mangling of LayoutRes/PAR/storage size. You must simply leave the file untouched, and use ass_set_storage_size and ass_set_pixel_aspect to the storage size and sample aspect ratio respectively. Only this way will the rendering match that of other players and hardsub filters.
If you are generating subtitles on the fly and want the text to look proportional when the video is displayed with its tagged DAR, then I'd recommend the following:
- still call
ass_set_storage_sizeandass_set_pixel_aspectto the correct values (so, in fact, set them unconditionally). - Set
d.ass->LayoutResXandd.ass->LayoutResYandd.ass->PlayResXandd.ass->PlayResYto the video's display resolution.
(Note that the second item makes the first item have no effect in this case, but it's good practice to set these values anyway, and it makes the code easier to reason about since they're just set unconditionally.)
IINM, not also setting the PlayRes to the display resolution may have been what caused the issues you've been seeing. So I'd expect that these two steps should be enough for proportional rendering, even if the d.ass->FooResX/Y values have to be changed when the PAR changes between frames, without needing any extra cache clearing. If that's not the case, I'll investigate further.
There was a problem hiding this comment.
If the user is expected to be able to supply complex ASS as input, then I think it would be more useful to set PlayRes to the physical amount of pixels in the video frame, so that things like coordinates, margins and vector shape widths make intuitive sense and don’t require the user to perform additional calculations and hope that they match the SAR calculations performed by SubText (considering questions like: does it keep the height or the width constant? does the answer depend on the AR? at the very least, you may want to document these answers and promise they’ll be stable going forward).
But this also affects border and shadow-offset widths, so if you want bog-simple plain-text subtitles to have symmetric/isotropic/nonanamorphic border thickness when the resulting video is eventually displayed and stretched back to square pixels by a video player, then you should indeed set PlayRes to have the intended ultimate display aspect ratio.
There was a problem hiding this comment.
OK, @arch1t3cht and @astiob is matching mpv's behavior for supplied .ass the "ideal path"? If so, this is what it would look like:
| Input | Frameprops read if no sar=? |
No sar= behavior |
With sar= |
|---|---|---|---|
Subtitle(text=...) |
Yes | Compensation follows frame props: layout size, PlayResX, and style margins track the SAR, so glyphs, blur, borders, spacing, drawings, and positions compensate as a unit |
Same full compensation at the fixed ratio; no clip frames are requested |
TextFile, convertible format (SubRip, WebVTT, etc.) |
Yes | Identical to Subtitle |
Identical to Subtitle |
TextFile, .ass with LayoutResX/LayoutResY |
No | libass uses file's LayoutRes | Ignored. LayoutRes outranks it. |
TextFile, .ass without LayoutRes |
No | Rendered in storage space (par=1), subtitles stretch with the picture downstream at display time | Opts in, renders as if file has LayoutRes headers for the display shape. Glyphs and blur become display-isotropic, file's PlayRes coordinates (position, margin, spacing, drawings) use storage-pixel meaning |
It seems wrong to not compensate and instead make it stretch later, but that looks like what mpv does with .ass format without LayoutRes by default. If that's the fansub community's expectation from the DVD era, we can match it.
I would differ from mpv and avoid using libass' par-setting function, instead using ass_set_storage_size to set LayoutRes so that blur-scaling is sane and the ASS_RenderPriv cache is still busted.
There was a problem hiding this comment.
Latest commit conforms to the table above.
|
Is this settled yet? |
Current master shouldn't be released. The insights from @arch1t3cht and @astiob are appreciated and indicate changes this PR should make (or put into a new PR). They'll need to be adapted for the VapourSynth video filter case where SAR in and SAR out are constant, different from the typical player case. I expect to make those changes this week. I just need to ensure that testing covers:
To answer @astiob's query, height is the dimension held constant, consistent with libass' inner workings. That can be added to the doc. |
Authored ASS files are left exactly as written, matching how other renderers treat them; generated Subtitle tracks and libavcodec conversions keep being compensated. Passing sar still opts a file in. Also scale PlayResX (and style margins) along with the layout size for our own tracks, so borders, spacing, drawings and positions compensate together with the glyphs instead of only the glyphs moving. Subtitle-generated scripts now set ScaledBorderAndShadow: yes
ass_set_pixel_aspectwas doing the opposite of intention and was stretching towards the target SAR instead of compensating for it in the burned-in subtitles. We could switch to supplying it the reciprocal of the SAR, but it looks like even with that it skips the layout calculations for any blur.This switches to using LayoutRes to specify a virtual display geometry at the target DAR. It sets it via
ass_set_storage_sizeto ensure cached geometry isn't used for subsequent frame requests (in case the next frame has a different SAR). It's oddly named, but seems to be what we want.A change is added to avoid compensating for SAR when LayoutRes comes from a subtitles file.