[Commited] [Core] SSA burn-in support

Developer discussion and patch submissions only!
Forum rules
This forum is for developer discussion and patch submission only.
davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Sun Sep 12, 2010 12:20 am

My guess is that it's fine.

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Tue Sep 14, 2010 7:33 am

I believe this is ready to be committed. Commit-devs?

User avatar
JohnAStebbins
HandBrake Team
Posts: 4993
Joined: Sat Feb 09, 2008 7:21 pm

Re: [Patch] [Core] SSA burn-in support

Post by JohnAStebbins » Tue Sep 14, 2010 4:25 pm

Build failed on linux. gcc doesn't have -arch flag and gtk ui libs were not updated. Updated patch:
http://handbrake.fr/pastebin/pastebin.php?show=1650

I also added the necessary bits to allow burning SSA with gtk ui. Untested since I have no files with SSA to test with.

User avatar
JohnAStebbins
HandBrake Team
Posts: 4993
Joined: Sat Feb 09, 2008 7:21 pm

Re: [Patch] [Core] SSA burn-in support

Post by JohnAStebbins » Tue Sep 14, 2010 4:56 pm

Of coarse, right after posting that patch, I broke it by committing the ffmpeg bump. Updated:
http://handbrake.fr/pastebin/pastebin.php?show=1651

User avatar
JohnAStebbins
HandBrake Team
Posts: 4993
Joined: Sat Feb 09, 2008 7:21 pm

Re: [Patch] [Core] SSA burn-in support

Post by JohnAStebbins » Tue Sep 14, 2010 10:40 pm

Found an mkv with ssa subs sample to test with. Fixed some issues in the gtk gui.
I also made a change in decssasub.c. David, you were modifying subtitle->format in the cli. The ui's and cli should only set what is in subtitle->config. So I modified the code to use subtitle->config.dest to decide whether to extract to text or render. The contents of the rest of the subtitle struct is information about the source. E.g. the source is a text based format.

http://handbrake.fr/pastebin/pastebin.php?show=1654

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Wed Sep 15, 2010 6:29 am

JohnAStebbins wrote:Build failed on linux. gcc doesn't have -arch flag and gtk ui libs were not updated.
Ah. Yet another virtual machine I need to setup: Fedora :-)

I have reverted two of your comment changes, because they seemed misleading to me. All code changes have been retained. New patch:
http://handbrake.fr/pastebin/pastebin.php?show=1655

Once support for either text burn-in or SSA picture passthru is added, we may want to revisit adding more information to subtitle->config to distinguish these cases.

User avatar
Rodeo
HandBrake Team
Posts: 11580
Joined: Tue Mar 03, 2009 8:55 pm

Re: [Patch] [Core] SSA burn-in support

Post by Rodeo » Wed Sep 15, 2010 11:24 am

davidfstr wrote:SSA picture passthru is added
You mean SSA -> VOBSUB conversion?

User avatar
JohnAStebbins
HandBrake Team
Posts: 4993
Joined: Sat Feb 09, 2008 7:21 pm

Re: [Patch] [Core] SSA burn-in support

Post by JohnAStebbins » Wed Sep 15, 2010 5:26 pm

One more tweak. The windows gui is going to need to be able to parse out the subtitle type so that it only allows burning of subs that support it. So I added the source to the string that gets printed for each sub.
http://handbrake.fr/pastebin/pastebin.php?show=1658

User avatar
Rodeo
HandBrake Team
Posts: 11580
Joined: Tue Mar 03, 2009 8:55 pm

Re: [Patch] [Core] SSA burn-in support

Post by Rodeo » Wed Sep 15, 2010 10:00 pm

Same patch as 1658, except I made SubSource an API function (becomes hb_subsource_name).

http://handbrake.fr/pastebin/pastebin.php?show=1662

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Thu Sep 16, 2010 6:27 pm

Rodeo wrote:
davidfstr wrote:SSA picture passthru is added
You mean SSA -> VOBSUB conversion?
I hadn't thought of that. Yet another possibility. :D

I was thinking MKV-SSA to MKV-SSA. But MKV-SSA to *-VOB also sounds interesting.

Added small tweak to return "UTF-8" instead of "UTF8" for UTF8SUB. This is consistent with the original CLI behavior:
http://handbrake.fr/pastebin/pastebin.php?show=1665

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Sun Sep 19, 2010 10:14 pm

So this patch has been tested under all of the supported build environments {mac, linux, mingw}. And on other developers' computers for all of these environments.

Commit?

User avatar
Rodeo
HandBrake Team
Posts: 11580
Joined: Tue Mar 03, 2009 8:55 pm

Re: [Patch] [Core] SSA burn-in support

Post by Rodeo » Sun Sep 19, 2010 10:28 pm

davidfstr wrote:Added small tweak to return "UTF-8" instead of "UTF8" for UTF8SUB. This is consistent with the original CLI behavior:
http://handbrake.fr/pastebin/pastebin.php?show=1665
Hope that was the only change, as this is not a patch. New patch with the change: http://handbrake.fr/pastebin/pastebin.php?show=1677

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Mon Sep 20, 2010 2:24 am

Rodeo wrote:Hope that was the only change, as this is not a patch. New patch with the change: http://handbrake.fr/pastebin/pastebin.php?show=1677
Hmm - no kidding. Must've copy/pasted from the wrong window. Thanks for updating that Rodeo.

LinBrake
Regular User
Posts: 97
Joined: Tue Oct 07, 2008 7:39 am

Re: [Patch] [Core] SSA burn-in support

Post by LinBrake » Mon Sep 20, 2010 11:54 pm

Is there a special procedure for applying the patches generated here? As far as I can tell the format is a unified diff, but the:

Index: test/test.c
===================================================================
--- test/test.c (revision 3548)
+++ test/test.c (working copy)

portion and similar seem to make the patch command choke on the diff.

User avatar
Rodeo
HandBrake Team
Posts: 11580
Joined: Tue Mar 03, 2009 8:55 pm

Re: [Patch] [Core] SSA burn-in support

Post by Rodeo » Tue Sep 21, 2010 12:54 am

patch -p0 works for me against trunk.

LinBrake
Regular User
Posts: 97
Joined: Tue Oct 07, 2008 7:39 am

Re: [Patch] [Core] SSA burn-in support

Post by LinBrake » Tue Sep 21, 2010 2:46 am

I just pulled the source down from SVN and it looks like it works.

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Tue Sep 21, 2010 6:33 am

Hey John, does this look ready to go in to you?

LinBrake
Regular User
Posts: 97
Joined: Tue Oct 07, 2008 7:39 am

Re: [Patch] [Core] SSA burn-in support

Post by LinBrake » Tue Sep 21, 2010 9:43 am

The patch works like a charm. SSA subtitles inside the MKV containers are made into perfect soft subtitles. I've tested this one dozens of files and not one problem.

User avatar
Rodeo
HandBrake Team
Posts: 11580
Joined: Tue Mar 03, 2009 8:55 pm

Re: [Patch] [Core] SSA burn-in support

Post by Rodeo » Tue Sep 21, 2010 9:53 am

LinBrake wrote:The patch works like a charm. SSA subtitles inside the MKV containers are made into perfect soft subtitles. I've tested this one dozens of files and not one problem.
That's completely unrelated to the patch. This patch is about hard subtitles from ASS/SSA sources.

User avatar
JohnAStebbins
HandBrake Team
Posts: 4993
Joined: Sat Feb 09, 2008 7:21 pm

Re: [Patch] [Core] SSA burn-in support

Post by JohnAStebbins » Tue Sep 21, 2010 1:16 pm

davidfstr wrote:Hey John, does this look ready to go in to you?
Yes, I think it's ready. I have limited time right now though. I'm traveling. So I would like to wait till I get back home. To be on the safe side, I would like to have time to deal with any issues that might come up after it's checked in and gets into the nightlies.
I'll be home next Tue.

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Tue Sep 21, 2010 3:24 pm

LinBrake wrote:The patch works like a charm. SSA subtitles inside the MKV containers are made into perfect soft subtitles. I've tested this one dozens of files and not one problem.
That's actually thanks to an older patch of mine, but I'm glad you enjoy it. :D

This new patch is for burned-in SSA subtitles, with most of the styling information (lacking animated effects like karaoke, fade in/out).

davidfstr
Regular User
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] SSA burn-in support

Post by davidfstr » Tue Sep 21, 2010 3:35 pm

JohnAStebbins wrote:Yes, I think it's ready. I have limited time right now though. I'm traveling. So I would like to wait till I get back home. To be on the safe side, I would like to have time to deal with any issues that might come up after it's checked in and gets into the nightlies.
I'll be home next Tue.
Okay. Thanks for the heads-up John.

psalento
Posts: 1
Joined: Tue Sep 21, 2010 5:02 pm

Re: [Patch] [Core] SSA burn-in support

Post by psalento » Tue Sep 21, 2010 5:36 pm

Patch seems to work.

However there is small issue with lowercase characters (j, p and g). With first two files I have tested those letters shadow is not working correctly. Most letter have black shadow and white text to up-and-left from the shadow. Most of the time those lowercase characters white parts are to down-and-left from the shadow.

I could post pictures but I am not sure whether this is the right thread. The problem might also be with font renderer that is used.

jamiemlaw
Veteran User
Posts: 536
Joined: Thu Sep 17, 2009 4:52 pm

Re: [Patch] [Core] SSA burn-in support

Post by jamiemlaw » Tue Sep 21, 2010 7:01 pm

Would someone mind helping me out? I'm not familiar with applying patches, and I can't seem to get this one to work.

So I did:

Code: Select all

svn co svn://svn.handbrake.fr/HandBrake/trunk handbrake-svn
cd handbrake-svn/
downloaded the diff file (1677.diff) and put it in there also. Then

Code: Select all

patch -p0 < 1677.diff
and

Code: Select all

./configure --launch --launch-jobs=0 ; open build/
then opened up the copy of HandBrake. I imported a MKV file that had some SSA subtitles, selected one of the tracks, but I can't choose to burn it in: the box remains greyed out. What am I doing wrong?

User avatar
Rodeo
HandBrake Team
Posts: 11580
Joined: Tue Mar 03, 2009 8:55 pm

Re: [Patch] [Core] SSA burn-in support

Post by Rodeo » Tue Sep 21, 2010 7:21 pm

Use the CLI or the LinGUI. The other UIs have not been updated to allow SSA burn-in.

Post Reply