Page 1 of 1

[Committed] [Core] Simultaneous subtitle support

Posted: Sat Oct 16, 2010 10:52 pm
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

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Sat Oct 16, 2010 11:45 pm
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).

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Thu Oct 21, 2010 7:09 am
by davidfstr
Updated path with more comments in fifo.c:
http://paste.handbrake.fr/pastebin.php?show=1771

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Sun Oct 31, 2010 4:52 am
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.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Wed Nov 10, 2010 2:35 am
by davidfstr
Finally back. Anyone had a chance to look at this?

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Wed Nov 10, 2010 3:15 am
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.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Wed Nov 10, 2010 6:17 am
by davidfstr
Of course. Work completely owned me last week.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Tue Jan 04, 2011 7:13 am
by davidfstr

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Sat Jan 15, 2011 7:45 pm
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.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Tue Feb 22, 2011 5:23 am
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.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Tue Feb 22, 2011 5:35 am
by Rodeo
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.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Tue Feb 22, 2011 7:40 am
by s55
If there are any problems, they can always be fixed post-commit, so go for it.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Thu Feb 24, 2011 9:52 am
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.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Thu Feb 24, 2011 4:24 pm
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.

Re: [Patch] [Core] Simultaneous subtitle support

Posted: Fri Feb 25, 2011 1:09 am
by davidfstr