GNOME Bugzilla – Bug 343348
[matroska] add support for vobsub subtitles
Last modified: 2006-09-04 16:30:01 UTC
That bug has been opened on https://launchpad.net/distros/ubuntu/+source/totem/+bug/47056
"It seems that Totem ignores vobsub subtitles in mkv files. Xine doesn't.
It would be great if the support for VobSub was not limited to Matroska files but also applied to external files (.idx + .sub) this is often the easiest way to get subtitles when making a DVD backup.
Created attachment 70211 [details] [review]
Patch implementing VOBSUB support in matroska-demux
Find a first attempt at this in the attached patch. This is my first Gstreamer code, I'd appreciate comments.
Vobsubs might be compressed in the matroska file. Thus this patch not only adds support for VOBSUB subtitles, but also for zlib compressed blocks.
A little hunk of the patch also uses the REFERENCEBLOCK data to skip non-key frames while seeking. This makes the life of the video and audio decoders easier (in my case, the ffmpeg XVid decoder emited a lot of warnings while seeking).
Something important to note: this patch is useless until someone fixes the dvdsubdec element to actually display the subtitles :-). The only thing that it will get you right now is SPU commands executed by dvdsubdec that you'll see in the element's debug output.
One last thing: while seeking with the patch applied, I get crashes in dvdsubdec. From time to time (but always while seeking), the element executes gst_dvd_sub_dec_merge_title() with a NULL dec->partialbuf. I don't know if it's an issue in dvdsubdec or in my patch, any idea?
2006-08-04 Frederic Riss <email@example.com>
* gst/matroska/matroska-ids.h: New defines for CONTENTENCODINGS
support and VOBSUB.
(struct _GstMatroskaTrackContext): Add encodings field.
* gst/matroska/matroska-demux.c (subtitle_src): Add mime type for
(gst_read_track_encodings): New function.
(gst_matroska_decode_buffer): New function.
Oops the patch leaks... I planed to add the memory freeing for the new GArray, but forgot to add it before posting :-(
I'll first wait for some comments and then send an updated patch if there's interest.
> Oops the patch leaks... I planed to add the memory freeing for the new GArray,
> but forgot to add it before posting :-(
> I'll first wait for some comments and then send an updated patch if there's
Patch looks really nice and clean, can't see much that needs changing at first glance, so please just go ahead and update your current patch :)
Created attachment 70222 [details] [review]
Matroska VOBSUB support without leak
Thanks for the quick review! Attached an updated patch without the memory leak.
I tried to add a 'depends' on bug 350044 which aims to fix dvdsubdec, but I don't have the rights to do so.
Committed with only a few minor style-related changes:
2006-09-04 Tim-Philipp Müller <tim at centricular dot net>
Patch by: Frédéric Riss <frederic.riss at gmail dot com>
* gst/matroska/matroska-demux.c: (gst_matroska_track_free),
Add support for VOBSUB subtitle tracks and zlib-compressed
tracks. Make sure we start on a keyframe after a seek. (#343348)
Many thanks for the patch!
(In reply to comment #2)
> One last thing: while seeking with the patch applied, I get crashes in
> dvdsubdec. From time to time (but always while seeking), the element executes
> gst_dvd_sub_dec_merge_title() with a NULL dec->partialbuf. I don't know if
> it's an issue in dvdsubdec or in my patch, any idea?
I've seen those as well, it was one of the reasons why I've set dvdsubdec's rank to NONE for the time being. I'm fairly sure that's a dvdsubdec issue though, since I've ran into it with dvddemux.
The more general playbin/Totem issue is already tracked in bug #350311, hence closing this bug.