[Committed] [Core] Simultaneous subtitle support

Archive of historical development discussions
Discussions / Development has moved to GitHub
Forum rules
*******************************
Please be aware we are now using GitHub for issue tracking and feature requests.
- This section of the forum is now closed to new topics.

*******************************
Post Reply
davidfstr
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

[Committed] [Core] Simultaneous subtitle support

Post by davidfstr »

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
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

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
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

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 »

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
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

Finally back. Anyone had a chance to look at this?
User avatar
JohnAStebbins
HandBrake Team
Posts: 5712
Joined: Sat Feb 09, 2008 7:21 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by JohnAStebbins »

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
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

Of course. Work completely owned me last week.
davidfstr
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

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

Re: [Patch] [Core] Simultaneous subtitle support

Post by s55 »

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
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

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.
Deleted User 11865

Re: [Patch] [Core] Simultaneous subtitle support

Post by Deleted User 11865 »

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: 10350
Joined: Sun Dec 24, 2006 1:05 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by s55 »

If there are any problems, they can always be fixed post-commit, so go for it.
davidfstr
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

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: 5712
Joined: Sat Feb 09, 2008 7:21 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by JohnAStebbins »

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
Enlightened
Posts: 149
Joined: Sun Apr 12, 2009 7:41 pm

Re: [Patch] [Core] Simultaneous subtitle support

Post by davidfstr »

Post Reply