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

libass build #2163

Merged
merged 18 commits into from
Jun 29, 2023
Merged

libass build #2163

merged 18 commits into from
Jun 29, 2023

Conversation

adipose
Copy link

@adipose adipose commented Jun 20, 2023

I removed the libass tags from this build and moved some logic into a subclass to clean up STS and RTS. Here's a test build.

https://mega.nz/file/RIx03CCK#c9IXdtUmKnuj_4tjFhM6N7tho3yAzDL12PEbCVX5M2k

At least a few people have expressed interest in this so maybe they can test it out.

@adipose
Copy link
Author

adipose commented Jun 22, 2023

I found a couple places that unloadass() had been removed since patch82--I added them back here, but maybe I shouldn't have. I remember some issue about that but I though you had fixed it by adding a check in SubtitleInputPin.cpp

@adipose
Copy link
Author

adipose commented Jun 22, 2023

@adipose
Copy link
Author

adipose commented Jun 22, 2023

Tests below. LIBASS left, MPC-HC on right:

Test 1: Results are the same except LIBASS is able to render outside the frame
Test 2: MPC-HC fails to render full string (.ass subtitle using Frank Reaction font we solved before with freetype)
Test 3: Both succeed
Test 4: MPC-HC fails with upside-down GDI glitch (note, it's also smaller..., maybe this is an overflow issue)
Test 5/6: LIBASS throwing a parse error. Otherwise both can render about the same. It looks as if LIBASS is a bit larger here.
Test 7: .ass sub: MPC-HC fails to render serbian glyphs, only shows Russian
Test 8: .srt sub: MPC-HC fails to render serbian glyphs, only shows Russian. SRT seems to be imposing a default font as well that's not picked up by libass
Test 9: This is a file with huge amount of text. Both handle it ok, and both render outside the frame
Test 10: Longer serbian test, MPC-HC cannot render glyphs properly. MPC-HC wraps within frame, but libass goes outside
Test 11: Original Frank Reaction test, identical results. MPC-HC is using Freetype path to achieve this.

libass-samples

@adipose
Copy link
Author

adipose commented Jun 22, 2023

Test 5 issue has been fixed. The problem was being reported by mpc-hc, not libass. The libass was failing due to wide chars in the filename, which is "broken" in libass on windows even with utf8 filename.

Also added an Empty() call to make it start fresh if libass fails. That's what caused the error.

@clsid2
Copy link
Owner

clsid2 commented Jun 24, 2023

It seems to render relative to the window instead of the video. Or at least using a wrong res.
sample
m_storageRes gets set to default 1920x1080 (using SrtResX/Y). It should be set to the encoded size of the video (before any anamorphic stretch).

@adipose
Copy link
Author

adipose commented Jun 24, 2023

It seems to render relative to the window instead of the video. Or at least using a wrong res.

sample

m_storageRes gets set to default 1920x1080 (using SrtResX/Y). It should be set to the encoded size of the video (before any anamorphic stretch).

It does use the spd but it allows render in the full window. I tried some options but nothing behaves quite like mpchc.

@clsid2
Copy link
Owner

clsid2 commented Jun 24, 2023

With the sample above, if you increase window height to add black bars, then it wrongly renders the sub in black bar.

@K0stov
Copy link

K0stov commented Jun 25, 2023

Thanks for continuing to work on this.

@adipose
Copy link
Author

adipose commented Jun 27, 2023

I rewrote the flattener code to limit the subs to the video window properly. However, I do seem to remember one mod we did to subtitles a few years back, to allow them to render below the video in some cases, like fullscreen with letterboxing? I bet that won't work without some mods.

All cases above now work correctly, and in some cases only libass can get correct results (upside down TEST should no longer be a problem in mpc-hc renderer).

image

image

@adipose
Copy link
Author

adipose commented Jun 27, 2023

Seems like those old letterbox issues were related to PGS, so maybe it doesn't matter.

@clsid2
Copy link
Owner

clsid2 commented Jun 27, 2023

Usage of libass should be restricted to ASS/SSA subs. At least initially, to avoid regressions with other formats.
Or maybe if libass+freetype options are both enabled, then use libass for all.

ASS/SSA should always be positioned relative to the video frame. So adding black bars by resizing window height should not have any effect on the subtitle position or size. Current code seems correct now for that.
We can make an exception for super simple scripts (like converted from SRT) that contain no positioning or animation tags. In that case we could allow user to override positioning. But we can look into that at later moment. Not important right now.

SRT and other formats can have positioning relative to the window. "position subtitles relative to video frame" option not fully checked. So in that case SetFrameSize should probably be set to full video window size when using libass.

What is the behavior of VLC/MPV for the test 1 sample? Do they render outside or cut off?

Regarding test 2. I though rendering the full string was fixed previously? Maybe regression due to recent freetype changes?

Can you make a fresh test build?

@adipose
Copy link
Author

adipose commented Jun 27, 2023

Usage of libass should be restricted to ASS/SSA subs. At least initially, to avoid regressions with other formats.
Or maybe if libass+freetype options are both enabled, then use libass for all.

What about two boxes--"use libass for substation alpha subs", "use libass for srt subs" ?

SRT and other formats can have positioning relative to the window. "position subtitles relative to video frame" option not fully checked. So in that case SetFrameSize should probably be set to full video window size when using libass.

OK, I'll take a look at that.

What is the behavior of VLC/MPV for the test 1 sample? Do they render outside or cut off?

Cut off.

Regarding test 2. I though rendering the full string was fixed previously? Maybe regression due to recent freetype changes?

I'm not sure yet why it fails. It seems to actually work with recent freetype codepath, but as of 2.0.0 release, it was broken. However, the original test (test11 above) did work with the same font.

@clsid2
Copy link
Owner

clsid2 commented Jun 27, 2023

"Use libass for SSA/ASS"
"Use libass for SRT"

@adipose
Copy link
Author

adipose commented Jun 27, 2023

@adipose
Copy link
Author

adipose commented Jun 27, 2023

Regarding test 2. I though rendering the full string was fixed previously? Maybe regression due to recent freetype changes?

The solution only worked if the path returned zero points. In this case, ABCDEFG... did return a path that was comprised of the points from A only. So although the path was wrong, it did have a partial path and did not fail over to the freetype path code.

@clsid2
Copy link
Owner

clsid2 commented Jun 27, 2023

Can you check if there is an issue with the test build? It looks like libass isn't being used, despite the option being enabled.

I use this sample to test because I know that has issues with libass.

@adipose
Copy link
Author

adipose commented Jun 27, 2023

Can you check if there is an issue with the test build? It looks like libass isn't being used, despite the option being enabled.

I use this sample to test because I know that has issues with libass.

Hmm, no it is working for me with the checkbox checked. If I uncheck it, I get the old behavior.

@clsid2
Copy link
Owner

clsid2 commented Jun 28, 2023

Semi-checked means relative to video for SSA/ASS and relative to window for everything else.

@adipose
Copy link
Author

adipose commented Jun 28, 2023

Semi-checked means relative to video for SSA/ASS and relative to window for everything else.

I meant a video/screenshot of what the output should look like with SRT and it being semi-checked.

@clsid2
Copy link
Owner

clsid2 commented Jun 29, 2023

For SRT semi-checked is same as unchecked. You can see behavior by using ISR with libass disabled.

@adipose
Copy link
Author

adipose commented Jun 29, 2023

For SRT semi-checked is same as unchecked. You can see behavior by using ISR with libass disabled.

This is implemented.

@clsid2 clsid2 merged commit 2a9e1da into clsid2:develop Jun 29, 2023
@clsid2
Copy link
Owner

clsid2 commented Jun 29, 2023

External tracks are working. But embedded ones not.
Problem is in CSubtitleInputPin::CompleteConnect. pRTS->m_SSAUtil.m_renderUsingLibass is false there. If I force it on, there is an issue with styles.

Sample with embedded tracks.

@adipose
Copy link
Author

adipose commented Jun 29, 2023

image

I tested this one with the default track--this doesn't work for you?

@clsid2
Copy link
Owner

clsid2 commented Jun 29, 2023

It works for you because it isn't using libass.
Set a breakpoint in for example SSAUtil::Render.

@adipose
Copy link
Author

adipose commented Jun 30, 2023

It works for you because it isn't using libass. Set a breakpoint in for example SSAUtil::Render.

OK, it's now rendering with libass and reloading correctly for mkv tracks.

However, fonts are bad...

#2174

image

@adipose
Copy link
Author

adipose commented Jun 30, 2023

Well, maybe it's a good thing, because it replicates the VLC behavior per here:

#358 (comment)

Hmm, forgot about this.

@clsid2
Copy link
Owner

clsid2 commented Jun 30, 2023

Working better now. That font bug is why this sample is a good way to test if libass is active ;)

Some issues with layoutres:
https://www.sendspace.com/file/ij6ewi
Not directly related to this, but I think setting storageres to a default value in SSAUtil::LoadASSFile is wrong. Only correct value is the size of the video. To be more specific the size before anamorphic stretching. So for example video is encoded as 720x576 and displayed as 1024x576. Also, does libass even use this variable?

Mem leak related to fonts:
https://www.sendspace.com/file/jdcmy9
VS reports leaks of Arial related fonts. So a default or fallback style that is used internally?

Are there any config settings for libass that might have effect on performance? Like cache sizes, etc. I have yet to test performance on some super heavy samples where ISR struggles.

@clsid2
Copy link
Owner

clsid2 commented Jun 30, 2023

Heavy animated sample:
https://www.sendspace.com/file/h136mq

CPU usage is much higher with libass :(

@adipose
Copy link
Author

adipose commented Jun 30, 2023

Mem leak related to fonts:

https://www.sendspace.com/file/jdcmy9

VS reports leaks of Arial related fonts. So a default or fallback style that is used internally?

My default is Calibri, what's yours? It could be a default font issue for sure.

@clsid2
Copy link
Owner

clsid2 commented Jun 30, 2023

Also Calibri.
I think I saw issue with non-Arial fonts as well. Need to run more tests.

@adipose
Copy link
Author

adipose commented Jun 30, 2023

Also Calibri. I think I saw issue with non-Arial fonts as well. Need to run more tests.

I couldn't get the mem leak--how does it manifest?

@adipose
Copy link
Author

adipose commented Jun 30, 2023

https://github.com/clsid2/mpc-hc/pull/2177/files

See if that helps. Not sure if it will do anything extra, because I didn't set any style overrides, but it does more than just clear fonts.

@adipose
Copy link
Author

adipose commented Jun 30, 2023

https://github.com/clsid2/mpc-hc/pull/2177/files

See if that helps. Not sure if it will do anything extra, because I didn't set any style overrides, but it does more than just clear fonts.

Never mind, it was already being called elsewhere.

@adipose
Copy link
Author

adipose commented Jul 1, 2023

pinterf/xy-VSFilter#28

Maybe worth checking for performance.

@K0stov
Copy link

K0stov commented Jul 1, 2023

Will the OpenType language setting be available when not using libass? It would be quite useful to me, an average viewer who usually sticks to .srt and prefers the special symbols of his native language.

@adipose
Copy link
Author

adipose commented Jul 1, 2023

Will the OpenType language setting be available when not using libass? It would be quite useful to me, an average viewer who usually sticks to .srt and prefers the special symbols of his native language.

I believe my freetype patch was merged already, which used the same setting together with the advanced freetype option.

Unfortunately I don't think it works great in all cases. It renders individual characters instead of a string which may affect spacing at times.

The test builds I made recently should have the advanced option.

@adipose
Copy link
Author

adipose commented Jul 1, 2023

Will the OpenType language setting be available when not using libass? It would be quite useful to me, an average viewer who usually sticks to .srt and prefers the special symbols of his native language.

I should also mention, in your case I would recommend using the "libass for SRT" option. If you have simple subs it should work fine and it does support opentype. The advanced freetype option also may work, but I probably need to do some more work to make that sophisticated enough to handle full strings.

@adipose
Copy link
Author

adipose commented Jul 2, 2023

Heavy animated sample: https://www.sendspace.com/file/h136mq

CPU usage is much higher with libass :(

Profiler shows a lot of time in the flatten sub. Not sure if this really helps much but I tried to optimize the calculations.

#2178

@clsid2
Copy link
Owner

clsid2 commented Jul 2, 2023

There is sse2 code for it here, which might help as well:
https://github.com/Masaiki/xy-VSFilter/blob/xysubfilter_libass/src/filters/transform/vsfilter/SubFrame.cpp

@clsid2
Copy link
Owner

clsid2 commented Jul 2, 2023

With this sample subs don't show up initially. They start working after a seek. I have not yet investigated what is wrong. It is also a very performance heavy sub for the translation part on the phone.

https://www.sendspace.com/file/sqasvt

@adipose
Copy link
Author

adipose commented Jul 2, 2023

There is sse2 code for it here, which might help as well: https://github.com/Masaiki/xy-VSFilter/blob/xysubfilter_libass/src/filters/transform/vsfilter/SubFrame.cpp

https://github.com/clsid2/mpc-hc/pull/2180/files

Didn't seem to make much difference.

@adipose
Copy link
Author

adipose commented Jul 9, 2023

Heavy animated sample: https://www.sendspace.com/file/h136mq

CPU usage is much higher with libass :(

Have you noticed--if we disabled subs entirely on this video, it still lags...?

@clsid2
Copy link
Owner

clsid2 commented Jul 9, 2023

The Ctrl+J graph is smooth here with subs disabled and MPCVR.

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.

3 participants