FS#7252 - SubRip (srt) subtitle support in MPEG Player. (Edit: +overlay timebar)

Attached to Project: Rockbox
Opened by Antoine Cellerier (dionoea) - Saturday, 02 June 2007, 14:18 GMT
Last edited by Antoine Cellerier (dionoea) - Sunday, 14 October 2007, 20:49 GMT
Task Type Patches
Category Video
Status New
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 2
Private No


This patch adds SubRip subtitle support to MPEG Player.

Files need to have the same name as the video file with a ".srt" extension.

I know that the OSD rendering is not implement in a way that linuxstb and jhMikeS would like but I still wanted to post the patch here in case anyone felt like adapting it. (Current OSD only renders on top of the video area, it can be adapted to add other OSD channels ... like display a seek bar, volume control, playback state, etc.)


Edit: I've also added a timebar display (when changing the volume)
This task depends upon

Comment by Anonymous Submitter - Saturday, 02 June 2007, 20:30 GMT
I miss fast forward and rewind more than every other feature like sub titles...but it sounds nice.
Comment by Pablo (pencha) - Friday, 20 July 2007, 22:34 GMT
I just posted a request for this!!
then I discovered the Patches section and found this!
I´ll try it as soon as I discover how to install it
Comment by Andrea Minini Saldini (gorman) - Thursday, 23 August 2007, 14:55 GMT
Is this available in any precompiled build? Does it work with recent versions of the code?
Comment by Martin H*** (webtaz) - Friday, 21 September 2007, 16:35 GMT
Has to be synced, i think.
Comment by Antoine Cellerier (dionoea) - Sunday, 30 September 2007, 16:15 GMT
Updated patch to make it apply with latest SVN (r14918).
Comment by benoit (snk4ever) - Tuesday, 09 October 2007, 14:37 GMT
Can it be applied together with the new mpegplayer ?
It fails for me (applying the 7487 before this patch).
Comment by Antoine Cellerier (dionoea) - Thursday, 11 October 2007, 12:39 GMT
I can't tell if it will ... depends if those two patches change the same lines of code or not. (+ due to recent mpegplayer changes, the patch might need a re-sync. I'll probably update it this week end)
Comment by benoit (snk4ever) - Thursday, 11 October 2007, 12:42 GMT
ok, I'll keep looking at your updates and try again !
Comment by Antoine Cellerier (dionoea) - Sunday, 14 October 2007, 15:15 GMT
Updated to work with revision 15105.
Comment by Antoine Cellerier (dionoea) - Sunday, 14 October 2007, 17:46 GMT
Added a time bar display (enabled for 3 seconds when any volume related button is pressed ... I didn't want to add another button).
Comment by Antoine Cellerier (dionoea) - Sunday, 14 October 2007, 20:48 GMT
Fix date function used. (Fixes subs when starting the video after seeking)
Comment by benoit (snk4ever) - Monday, 15 October 2007, 11:42 GMT
I tried to apply it, it seems to successfuly patch but doesn't compile.
I patch from the /path_to_source/apps/plugins/mpegplayer directory and when compiling I get this error :

subtitles.c: In function `uint8_buffer_putsxyofs':
subtitles.c:320: error: too few arguments to function
subtitles.c:328: error: too few arguments to function
make[3]: *** [/home/user/rockbox15TEC/testBuid/apps/plugins/mpegplayer/subtitles.o] Error 1
make[2]: *** [mpegplayer] Error 2
make[1]: *** [rocks] Error 2
make: *** [build] Error 2
Comment by Antoine Cellerier (dionoea) - Monday, 15 October 2007, 12:14 GMT
Was that applied on a clean svn checkout? (Are you using something like the multifont patch? I've seen reports on the unsupported builds forum that my patch doesn't work with it and your error seems to be on font related functions)
Comment by benoit (snk4ever) - Monday, 15 October 2007, 12:29 GMT
I'm trying on a clean SVN right now
Comment by benoit (snk4ever) - Monday, 15 October 2007, 12:43 GMT
OK, this time it compiles, but the simulator "segmentation-faults" when I launch a video, maybe it is usual on the simulator.
I'll try to combine it with other patches, excluding the multifont.
Comment by Antoine Cellerier (dionoea) - Monday, 15 October 2007, 13:01 GMT
It worked fine on the iPod Video simulator yesterday (well ... I had segfaults some of the time when changing the start point, not sure if they're subs related though)

Edit: you could try runing the sim in gdb to see where it segfaults (and post a backtrace here ... or fix it yourself :) )
Comment by benoit (snk4ever) - Monday, 15 October 2007, 15:08 GMT
What device are you using for your tests ?
I just tried on my Gigabeat F40 for real and it displayed the 1st sentence of my .srt file. Then at the time of displaying the 2nd, the device froze totally.
I can give you the mini-clip and srt file, I did the srt myself just for testing, it's some crap text :
Comment by benoit (snk4ever) - Monday, 15 October 2007, 15:10 GMT
I don't knwo if this has an importance but the file was made with a unix carriage return software.
Comment by Antoine Cellerier (dionoea) - Monday, 15 October 2007, 15:19 GMT
The subs file looks valid. I'll give it a try when I get back home (the only issue might be a segfault when the last sub line is read ... since I never really tested it all the way to the end of the movie).
Comment by Antoine Cellerier (dionoea) - Monday, 15 October 2007, 20:12 GMT
This patch add lots of error checking in the subs parsing function (which fixes all the segfaults I was able to come up with while editing the subtitles file).
It also restricts overlay rendering to normal video playback (== it's disabled for video thumbnails when setting the start time)

I guess that the next step(s) would be to:
* Make it possible to configure a delay (in case subs and video/audio aren't exactly synced)
* Make it possible to choose the subtitle's position on the video (ie: at the bottom of the screen instead of at the top ...)
* ...

Edit: Updated patch to make sure that the last line was always displayed. (Nice test video btw :) )
Comment by Antoine Cellerier (dionoea) - Monday, 15 October 2007, 21:23 GMT
About the multifont patch, if you're applying it with this patch you'll need to add a 3rd argument of rb->get_lcdgetcurfont() (or maybe just 0) to the two functions which trigger a compile error (font_get_bits and font_get_width).
Comment by Antoine Cellerier (dionoea) - Tuesday, 30 October 2007, 16:59 GMT
Benoit, have you been able to test the patch again? (and get it to work correctly)
Comment by Phil Light (phillight) - Saturday, 15 March 2008, 21:14 GMT
Synced to current SVN.

Currently when seeking in a file, you will see all subtitles up to that point in the video quickly flash by. This should be very simple to fix.
Comment by Phil Light (phillight) - Saturday, 15 March 2008, 21:42 GMT
Oops. Now based on the patch from 15th Oct, with timebar removed. Works correctly when seeking as well.
Comment by Marcoen Hirschberg (marcoen) - Saturday, 28 June 2008, 19:56 GMT
Just tried it. Works pretty well, although I would prefer the text on the bottom of the screen and lines longer than the screen width are broken off. Maybe I should work on it :)