GNOME Bugzilla – Bug 525743
support for kate subtitle streams
Last modified: 2009-08-08 11:58:57 UTC
Hi, I am working on a patch to add a kate decoder to -bad, now working in Totem. Would you be interested in such a patch ? Still needs some work before I can submit it. More information about the format is at: http://wiki.xiph.org/index.php/OggKate Thanks
Of course, feel free to put here a patch when you are ready...
Created attachment 108534 [details] [review] adds a decoder and parser for kate streams Adds preliminary decoder and parser support for kate streams.
There is second patch, but I'm hitting a bugzilla internal error each time I try to submit it (reported to bugmaster). It's very small though, so here it is. Muxed streams (using oggmux from -base) need the second hunk to be correct. Reading the code, I think the increase of the pageno is wrong as it is ++'d afterwards at the ogg_stream_flush, but I'm not confident this is right. I've tried muxing a theora stream and a vorbis stream as well with this patch in, and oggz-validate (checks ogg streams for errors) is happy with them. Would be nice to have someone who groks oggmux well to have a look at it to see if I'm not missing anything there. The first hunk just allows oggmux to accept application/x-kate on its sink and is needed for the kate parser to be able to be piped to oggmux, diff -ru /tmp/gst-ogg/gstoggmux.c gst-plugins-base-0.10.17/ext/ogg/gstoggmux.c --- /tmp/gst-ogg/gstoggmux.c 2008-04-02 19:53:47.000000000 +0100 +++ gst-plugins-base-0.10.17/ext/ogg/gstoggmux.c 2008-04-02 21:47:18.000000000 +0100 @@ -89,7 +183,8 @@ GST_STATIC_CAPS ("video/x-theora; " "audio/x-vorbis; audio/x-flac; audio/x-speex; " "application/x-ogm-video; application/x-ogm-audio; video/x-dirac; " - "video/x-smoke; text/x-cmml, encoded = (boolean) TRUE") + "video/x-smoke; text/x-cmml, encoded = (boolean) TRUE; " + "application/x-kate") ); static void gst_ogg_mux_base_init (gpointer g_class); @@ -1253,7 +1346,7 @@ if (GST_BUFFER_IS_DISCONT (buf)) { packet.packetno++; /* No public API for this; hack things in */ - pad->stream.pageno++; + //pad->stream.pageno++; force_flush = TRUE; }
I think Kate's bitstream format / spec should be stabilised/finalised before this is added. Also: What programs create content in this format? Where can content in this format be found? Is there a community around this or are you the only person using this at the moment?
Though the wiki does not mention this, the bitstream is now versioned, so while not frozen, it is not a moving target. The format includes hooks to allow adding data without breaking old decoders (in fact, I already use one of those). Only one program (apart from the kate encoder supplied with the library) creates kate streams at the moment: the latest ffmpeg2theora (and this uses only unformatted text now). Content in this format may not be found yet as the format is quite new, and there being only one "real" program writing it, and no players yet having decoding capability. I am so far the only person using it, again due to the format being new.
Created attachment 113104 [details] [review] kate stream encoder/decoder/parser This is further work, adding an encoder, and tests, as well as cleanup and a few fixes.
Created attachment 113105 [details] [review] allows oggmux to mux kate streams This trivial patch allows oggmux to accept and mux kate streams. The previous flush hack is removed, it all works fine now.
Created attachment 113244 [details] [review] kate stream encoder/decoder/parser another update, adding encoding tags, color/contrast parsing fix, and more tests
Created attachment 114174 [details] [review] add a tagger, a couple encoding fixes, cleanup, more tests
Created attachment 116673 [details] [review] update, adds canvas size property
Created attachment 116674 [details] [review] update to HEAD These patches are ported to a recent HEAD. It would be nice if the trivial patch to -base was applied, as it should not cause any adverse effects, and would allow using an out of tree kate plugin without having to patch GStreamer's ogg plugin. It's a one liner adding the application/kate MIME type to the list of allowed inputs to the ogg muxer. Thanks
Created attachment 119797 [details] [review] update: better parser, rendering element parser now more resistant, add a rendering element based on libtiger (using cairo and pango)
Created attachment 119799 [details] [review] add demuxing from and muxing to matroska
Created attachment 119800 [details] [review] the one liner that allows oggmux to accept kate streams the one liner that allows oggmux to accept kate streams, would be nice to get this in if possible as it's non intrusive
Created attachment 124671 [details] [review] update to latest CVS, and some improvements to the renderer
Created attachment 124672 [details] [review] update to latest CVS
Created attachment 124673 [details] [review] update to latest CVS
Created attachment 125715 [details] [review] Fix use of C99 (decl after stmt) that made this stop building - no functional change otherwise just in case someone might have wanted to try applying it ^_^
Created attachment 126650 [details] [review] use (new ?) GST_ELEMENT_DETAILS, s/%d/%f/ in debug log, and now applies to latest HEAD
Created attachment 128786 [details] [review] as git patch, updated to latest head
Created attachment 128787 [details] [review] as git patch, update to latest head
Created attachment 128788 [details] [review] as git patch, updated to latest head
Could you please let us know your real name? We'd like to know our copyright holders and be able to attribute non-trivial patches correctly.
My name is Vincent Penquerc'h. I'd rather be attributed as my pseudonym (general passive defense against automated cross referencing of information), but if you have a preference for real names, that's fine by me.
Created attachment 130234 [details] [review] Update to latest to keep it applying
Hi, has anyone had a chance to look at this recently ? The GStreamer patch is essentially done, the last few iterations were just tweaks and updates to keep the patch applying. If there's anything that needs changing, answering, etc, please feel free to ask. Thanks
Sorry this is taking so long. I was meaning to get this in before the freeze and have committed it locally, but didn't have a chance to look over it or ensure basic functionality before the freeze. Will get it in as soon as things get unfrozen though.
Great, thanks. Any problem, let me know. Also if you need varied test streams.
Just for the avoidance of doubt: the patch hasn't been applying cleanly for a while now, but I haven't posted an update since you said it was committed locally - give me a shout if you'd rather get a patch that applies to the current master.
> Just for the avoidance of doubt: the patch hasn't been applying cleanly for > a while now, but I haven't posted an update since you said it was committed > locally - give me a shout if you'd rather get a patch that applies to the > current master. Sorry, I had it applied locally, but I lost that when my VM setup got wiped out a while back, so an updated patch set or a repo to pull from would be much appreciated. I'm hoping to find some time in the next two weeks to finally integrate this.
Created attachment 137614 [details] [review] against latest
Created attachment 137615 [details] [review] against latest
Created attachment 137616 [details] [review] against latest
All patches now updated against latest master, tests still pass. Phew :) Thanks
Any reason you chose 'application/x-kate' as GStreamer media type, rather than e.g. subtitle/x-kate or somesuch? I think I'd prefer the latter.
I thought these were MIME types, rather than GStreamer specific ? application was chosen because it's semantically (mostly) text, but not actual text, so text/x-kate was out. As for subtitle/x-kate, generic handling by some element that would try to use 'subtitle/*' wouldn't work due to the binary encoding, so there's not much gain to this. Moreover, subtitles are just one possible use. If this media type is GStreamer specific, then you could change it, but I think it would create confusion with the MIME type as saved in, eg, the Skeleton track by ffmpeg2theora.
Another question: so the encoder accepts both text/markup and dvd-subpictures; any reason why the decoder only outputs text/markup and not also dvd subpictures? Can the dvdspu element not also be used to overlay dvd-subpicture streams stored in kate on top of video?
It would be possible to do this, if code was added to convert images from a Kate stream to the DVD subpicture format (the SPUs are not just dumped as in in the stream, though the encoding used for SPU type images is also RLE based). Since the renderer in this element handles all types of images that may be found in a stream, I thought it unnecessary to have a decoder convert back paletted images to SPUs. Would it gain much to also do it this way ? (compatibility, speed ?)
> It would be possible to do this, if code was added to convert images from > a Kate stream to the DVD subpicture format (the SPUs are not just dumped > as in in the stream, though the encoding used for SPU type images is also > RLE based). Since the renderer in this element handles all types of images > that may be found in a stream, I thought it unnecessary to have a decoder > convert back paletted images to SPUs. Would it gain much to also do it > this way ? (compatibility, speed ?) Well, it seems a bit asymetrical as it is now. And I feel that one should be able to get to the subpicture data in some way (either as a fake kate bitmap format, for which one could have yet another decoder/transcoder that changes it into dvd format, or included in katedec) without overlaying it, e.g. for transcoding purposes. Also, for playbin integration it would be much much easier if katedec outputted dvd-subpicture format for bitmaps. I think we want to avoid plugging all kinds of overlay elements, or rows of overlay elements, in playbin. Besides, it seems your overlay element only handles RGB at the moment, which isn't really ideal for the still most common case where the decoder outputs YUV which gets piped to xvimagesink, as far as I can tell. (In the long run / for 0.11/1.0 we would probably want some sort of generic overlay format in gstreamer with one overlay element to rule them all; I think Jan might have started looking into that.)
I will have a look at this then. It'll just ignore everything in the stream apart from paletted images, but I suppose a fair number of streams would be just these anyway. Fake Kate bitmaps don't really sound like a good idea, PNG would be a better match instead. RGB is because Cairo doesn't do YUV, and rendering on top of subsampled YUV often doesn't look very nice anyway.
Ok, great. Do you expect that people will (or should?) create 'mixed type' streams, that is plain text or markup interleaved with bitmaps in the same kate stream?
It is definitely possible, and encouraged. Something which I was looking at is extraction from a PDF file, so slides from a conference can be embedded at the correct times within a stream with the A/V from the speaker. At the moment, it's all embedded as a large PNG image, but I was hoping to do text/image extraction at some point, so that'd be a Kate stream with text, images, and formatting (typically background+logo as images, and the talking points as text).
Right, in that case it'd be nice if there was some sort of 'purpose/use type' enum in the headers so we can skip these sort of streams and only present true subtitles/lyrics to the user (since I don't see us supporting stuff like this any time soon tbh; besides, I feel that things like slides should be presented as additional out-of-band info to the stream which players could show in addition in a separate window for example rather than have them overlayed, but that might just be me).
The 'category' property from katedec is that 'purpose/use type' you need. A list of well known category types is available at wiki.xiph.org/index.php/OggText In VLC, I fill the subtitles list menu with, for all streams: <language> <category> Category is a textual code, so may be translated in the user's language, so for category 'LRC' and language 'de', you'd get: German lyrics for an English user. As for the slides thing (category K-SLD-I, not in the well defined list for now), I have them displaying on a second window in VLC (in a local patch, it's unwieldy, and unfinished). Actual presentation would be up to the client, but I'd imagine that if the stream is enabled, new slides would go fullscreen for a few seconds when the presenter switches to them, then zooming out back to a corner to have them as a clickable zoom in/out icon would be a nice way to do it, for a video player wanting to do presentations. As a side note, if subtitles were also present, they'd typically be in a separate stream, as text.
Should I create a new bug with the x-dvd-subpicture when I get round to doing it, or just post it here (several things queued up just now and it's not a quick fix) ?
> Should I create a new bug with the x-dvd-subpicture when I get round to > doing it, or just post it here (several things queued up just now and > it's not a quick fix) ? Just file a new bug when it's ready (I'm trying to get the other stuff in before the freeze on Monday, but not making any promises, since I'm travelling at the moment).
Decoding/reencoding to DVD SPU now in: http://bugzilla.gnome.org/show_bug.cgi?id=588638
Ok, here it goes: commit 2f7ec508993fd8f8a5f3b343b5873ef09069154a Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sun Jul 19 23:45:02 2009 +0100 docs: add inspect info for kate plugin Should fix the docs build. commit 4728d7f18eff48760ff2e753142b7313eaf552d1 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sun Jul 19 23:35:05 2009 +0100 kate: add some FIXMEs commit faf2d0469691a0f357cf6a4c048cf774ddf5b990 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sun Jul 19 23:32:07 2009 +0100 katedec: demote to GST_RANK_NONE for now There are still some autoplugging issues to sort out, and it needs some testing. commit 0d16612717860fff76a9c1889d47f7f5fe0fbf7c Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sun Jul 19 23:29:19 2009 +0100 checks: add kate unit tests to valgrind blacklist for now And add check binary to ignore list. commit 3f347c7edd0796fb033bb58f15017f1f6c6c5ebe Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sun Jul 19 23:16:07 2009 +0100 kate: make sure to free some more stuff commit 71efbb1e73a557d50093ccaf2ecc47009b7edd95 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sun Jul 19 22:29:19 2009 +0100 kate: fix up for additional subtitle/x-kate media type commit 71e6bbd19fa1845dc07abc561b1ca5a71ae59d60 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Mon Jul 13 22:38:43 2009 +0100 kate: remove local kate typefinder, use the one in -base commit ce0d40b634a08aec047f8fd3fb059c1eb4480e1c Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Fri Jul 10 18:45:28 2009 +0100 kate: change media type to subtitle/x-kate and update define accordingly commit e88984ccbd4b2b7b5527cbc3ff488c5c85bb1e97 Author: Vincent Penquerc'h <ogg.k.ogg.k@googlemail.com> Date: Sun Feb 15 18:35:04 2009 +0000 add new Kate plugin, for Kate overlay streams katedec: Kate decoder (text only) kateenc: Kate encoder (text and DVD SPU only) katetag: Kate tagger kateparse: Kate parser tiger: Kate renderer using the Tiger rendering library Fixes #525743. I'm sure I've introduced an issue or two with the media type change (apologies in advance), but I don't think I'll find those any sooner by keeping the code on my harddrive. Also demoted the decoder to RANK_NONE for now until the subtitle/subpicture overlay stuff in playbin(2) is sorted out. It'd be great if you could make the unit tests valgrind clean at some point (there seem to be a few minor mem leaks, and also a jump-depends-onuninitialised-memory issue in the test_kate_encode_spu test). I've also had the encode_spu test segfault once, but never managed to reproduce that again. Btw, whenever you return GST_FLOW_ERROR, you should post some sort of error message on the bus, otherwise the user will just get to see a generic 'internal flow error' message from the source element or demuxer, and won't have the slightest idea where that comes from. Something like this should do the job: GST_ELEMENT_ERROR (encoder, STREAM, ENCODE, (NULL), ("debug message")); Thanks for the patches and your patience!
Sorry, hadn't run on Valgrind for ages, I'll fix it asap. If you have a sample that makes it crash (or not behave as it should), feel free to send it (or a minimal subset) to me. By the stuff in playbin, do you mean the dvdspu crash, or something else ? Thanks for your time!
Created attachment 138793 [details] [review] this avoids using trashed memory when a DVD uses an invalid end time This fixes the valgrind unitialized value report. About leaks, however, I don't see any that seem to be mine, though in the sea of spam it's hard to see. I'm using the supp file I found, and I'm still getting imperial craploads of stuff from other places, so it makes it hard to see anything. If you have a log which shows leaks, feel free to send it to me.
Created attachment 138805 [details] [review] SUB is now the preferred subtitles category, use this too.
Committed, thanks: commit a7c64556ff25eae0ca3325dbbe166352b9b03006 Author: Vincent Penquerc'h <ogg.k.ogg.k@googlemail.com> Date: Mon Jul 20 12:25:15 2009 +0100 kateenc: also recognise the new recommended 'SUB' category Move the check for 'simple' subtitles category to a separate routine and add in the new recommended SUB category (#525743). commit 9db1323d310d0fdb0a1e41b5278fc0c3fc84641e Author: Vincent Penquerc'h <ogg.k.ogg.k@googlemail.com> Date: Mon Jul 20 11:41:40 2009 +0100 kateenc: keep bitmap/palette/region around when on the spot encoding is not possible due to an unknown end time Fixes valgrind unitialized value report. See #525743.
Created attachment 138816 [details] make elements/kate.valgrind log
Spooky, I double check my log (done this morning), and I don't see these leaks. Thanks, I'll check those.
Created attachment 138826 [details] [review] Fixes leaks found by Valgrind on elements/kate.c Found the way to use Valgrind from within the build sytem, I was running valgrind --leak-check=full --show-reachable=yes elements/kate, which was giving me lots of extra spam (even with the supplied supp file) and not the ones in the log above (lack of --trace-children=yes I suppose). No leaks left in that test.
Great, thanks!
Created attachment 138902 [details] [review] Moving/factoring stuff around with the recent SPU creation code caused me to introduce a bug in the time conversion macro Without this, SPU subtitles would be encoded with zero duration.
> Btw, whenever you return GST_FLOW_ERROR, you should post some sort of error > message on the bus, otherwise the user will just get to see a generic 'internal > flow error' message from the source element or demuxer, and won't have the > slightest idea where that comes from. Something like this should do the job: > GST_ELEMENT_ERROR (encoder, STREAM, ENCODE, (NULL), ("debug message")); I most often have a GST_OBJECT_WARNING at these places. Should GST_ELEMENT_ERROR replace those, or come in addition (eg, would logs be generated from calls to GST_ELEMENT_ERROR to help in parsing log files ?)
> I most often have a GST_OBJECT_WARNING at these places. Should > GST_ELEMENT_ERROR > replace those, or come in addition (eg, would logs be generated from calls to > GST_ELEMENT_ERROR to help in parsing log files ?) They should replace the GST_{WARNING|ERROR}_OBJECT. GST_ELEMENT_ERROR will log its own GST_WARNING_OBJECT messages (with the right file/line info, since it's a macro).
Created attachment 139182 [details] [review] Use GST_ELEMENT_ERROR as requested
> Use GST_ELEMENT_ERROR as requested Thanks! Committed with some changes (mostly just used (NULL) for the main error message and moved your untranslated strings into the debug message part of the error, since that's not really stuff we should show to the user); also changed one or two GST_FLOW_ERRORs into GST_FLOW_NOT_NEGOTIATED (where caps are missing or could not be set). commit aaed93e1265959bb06c4759e68537788c9446681 Author: Vincent Penquerc'h <ogg.k.ogg.k@googlemail.com> Date: Fri Jul 24 21:54:59 2009 +0100 kate: use GST_ELEMENT_ERROR for error reporting See #525743.
Also committed this, with some minor changes (for the gstreamer media type change, but also leave the xiph3 header packing function as it is now to minimise risk of breakage two days before code freeze): commit 19b7001bf99ea37a91af6061ca98e50b08064017 Author: Vincent Penquerc'h <ogg.k.ogg.k@googlemail.com> Date: Sun Feb 15 18:49:44 2009 +0000 matroska: add kate subtitle support to matroska muxer and demuxer See #525743.