[Committed] [Core] Simultaneous subtitle support

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

[Committed] [Core] Simultaneous subtitle support

Post by davidfstr » Sat Oct 16, 2010 10:52 pm

This patch adds support for temporally overlapping subtitles.

Major changes are concentrated in the sync and render work-objects.
  • The sync work-object's handling of subtitles has been completely rewritten, since my changes were too difficult to fit into the old algorithm. I have left the old algorithm in for the moment, since the patch becomes very difficult to read when it is removed outright. While this new algorithm is being tested it may also be useful to reenable the old algorithm occasionally for comparison.
  • The render work-object now supports queueing a list of subtitles to be processed for each frame. To implement this, FIFOs have been extended to support pushing/popping buffer-lists as single elements.
The new sync work-object maintains a list of subtitles in (sync->sub_list) that are popped from the subtitle raw queue. This list is all of the subs that could apply to the current frame or upcoming video frames. This list is repeatedly scanned for subtitles that apply to the current video frame that is being passed through.

While working on this I discovered there are some subtitle timing issues when the subtitle decoder lags behind the video decoder, which seems to be very common with the SSA subtitle decoder. (You can see it by enabling the new flag SUBSYNC_VERBOSE_TIMING.) Will investigate and fix in a separate patch.

The patch:
http://paste.handbrake.fr/pastebin.php?show=1761

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Sat Oct 16, 2010 11:45 pm

I should mention there are a few observable behaviors changed/removed when comparing the old to the new subtitle-sync algorithm:
  • Temporally overlapping subtitles are no longer trimmed to be non-overlapping. (Duh.) This includes DVD subtitles.
  • Subtitles less than two seconds long are no longer artificially extended. Sorry, Indochine fans.
  • Subtitles that stop before they start will never be displayed. The old algorithm will display such subtitles if they begin in the future (relative to the current video frame being processed).

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Thu Oct 21, 2010 7:09 am

Updated path with more comments in fifo.c:
http://paste.handbrake.fr/pastebin.php?show=1771

eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Re: [Patch] [Core] Simultaneous subtitle support

Post by eddyg » Sun Oct 31, 2010 4:52 am

Sorry about the two second thing. Maybe I'm just too slow at reading!

Although indochene was a worst case. I don't know what they were thinking hen having a couple of sentences up for less than two seconds.

Well done on all your work on subtitle support, you're doing a great job.

Cheers Ed.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Wed Nov 10, 2010 2:35 am

Finally back. Anyone had a chance to look at this?

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by JohnAStebbins » Wed Nov 10, 2010 3:15 am

I've taken a quick look at it and have some reservations about the fifo changes. I need to take a much closer look in order to understand what's going on. Once I really grok the intent, I may suggest some changes. I'm sorry I haven't done this already, but work is kicking my a-s-s lately.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Wed Nov 10, 2010 6:17 am

Of course. Work completely owned me last week.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Tue Jan 04, 2011 7:13 am


User avatar
s55
HandBrake Team
Posts: 8812
Joined: Sun Dec 24, 2006 1:05 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by s55 » Sat Jan 15, 2011 7:45 pm

Run some testing, haven't seen any regressions here with regards to SSA encodes.

Pushing thread back up to the top of the dev forum for comments.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Tue Feb 22, 2011 5:23 am

Since I have received no additional comments, I will be committing this on Wednesday. This will fix the most glaring SSA issue at present.

Speak now or forever hold your peace.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by Rodeo » Tue Feb 22, 2011 5:35 am

davidfstr wrote:Since I have received no additional comments, I will be committing this on Wednesday. This will fix the most glaring SSA issue at present.

Speak now or forever hold your peace.
FWIW, John is absent ATM. So he may not be able to comment right away.

User avatar
s55
HandBrake Team
Posts: 8812
Joined: Sun Dec 24, 2006 1:05 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by s55 » Tue Feb 22, 2011 7:40 am

If there are any problems, they can always be fixed post-commit, so go for it.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Thu Feb 24, 2011 9:52 am

Reran some of my input files on this change and saw a lot of lag in the subtitle syncing (over 1000ms). Did some investigation and traced the problem to a bug in the 'sub->sequence > cur->sequence' logic. Subtitle lag times are now around 20-40ms on average.

Will be hammering on this changelist with a few more inputs before it goes in.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by JohnAStebbins » Thu Feb 24, 2011 4:24 pm

David, I think it's ok to commit it when you are happy with it. The issues I have are mostly non-functional design issues that can be cleaned up in future passes.

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr » Fri Feb 25, 2011 1:09 am


Post Reply