HandBrake

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

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).

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

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.

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

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.

Of course. Work completely owned me last week.

Patch updated for 0.9.5:
http://paste.handbrake.fr/pastebin.php?show=1985

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.

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.

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.

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

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.

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.

Committed: https://trac.handbrake.fr/changeset/3804