Skip to content
This repository has been archived by the owner on May 12, 2024. It is now read-only.

Question about libass renderer or patches #54

Closed
TheOneric opened this issue May 12, 2022 · 4 comments
Closed

Question about libass renderer or patches #54

TheOneric opened this issue May 12, 2022 · 4 comments

Comments

@TheOneric
Copy link
Contributor

TheOneric commented May 12, 2022

I noticed something odd about your builds and was wondering whether you happen to have an idea of what’s going on. If not it’s hopefully probably harmless and fine to leave as is.

I was looking at backporting wangqr/Aegisub#134 to fix the libass backend to AegisubDC, but surprisingly the r9215B binary somehow already renders the sample (see linked PR) correctly — eventhough the current sourcecode here on GitHub does never call ass_set_storage_size. The r9214 binaries still render it as expected (i.e. wrong).

I looked a bit through the code but didn't find anything explaining why the rendering appears correct with r9215. Did you apply some other patch to the vendored libass which might cause this? Potentially such a patch could have detrimental effects on other samples even if it works out well here. Or maybe you did apply the same patch as in the linked PR for the binary build and just forgot to push it to GitHub?

To visualise, here’s how it looks in r9214 and how I'd exepect r9215B to look based on the source:
aegisubdc_9214

But here's what r9215B actually looks like and how I'd expect the current source to look after the patch has been applied:
aegisubdc_9215

Note: the sample is intended for use with DejaVu fonts in case you want to exactly reproduce it

@Ristellise
Copy link
Owner

Ristellise commented May 13, 2022

Hi! I've checked my git source locally and I do have the patch from that commit:

void LibassSubtitlesProvider::DrawSubtitles(VideoFrame &frame,double time) {
	ass_set_frame_size(renderer(), frame.width, frame.height);
	// Note: this relies on Aegisub always rendering at video storage res
	ass_set_storage_size(renderer(), frame.width, frame.height);

I just haven't pushed it at all but it is in alright.

Although I have a cursed version of libass being built, I can say that for 9215B, there is no modifications to libass source anymore.

@TheOneric
Copy link
Contributor Author

Great, this solves this mystery then and there's no need to backport the patch if it's already applied. No libass-modifications is also good news. Thanks for looking into this!

@astiob
Copy link

astiob commented Jun 8, 2022

Sorry to be a bother, but am I guessing correctly that the patch is in the last binary but still not in the GitHub source repo? Could you perhaps push it? Just so, you know, the public sources match the binaries.

@Ristellise
Copy link
Owner

Sorry to be a bother, but am I guessing correctly that the patch is in the last binary but still not in the GitHub source repo? Could you perhaps push it? Just so, you know, the public sources match the binaries.

Will do as soon as I get around it. My focus for sub editors is starting to drift towards Ameko, so yeah.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants