After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 733819 - Port teletextdec to 1.0
Port teletextdec to 1.0
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 734002 (view as bug list)
Depends on:
Blocks: 755665
Reported: 2014-07-27 13:36 UTC by Daniel Kamil Kozar
Modified: 2016-11-27 12:29 UTC
See Also:
GNOME target: ---
GNOME version: ---

Patch porting teletextdec to 1.0 (35.78 KB, patch)
2014-07-27 13:36 UTC, Daniel Kamil Kozar
needs-work Details | Review
Change caps to application/x-teletext, don't include gstatomicqueue.h (1.71 KB, patch)
2014-07-27 15:47 UTC, Daniel Kamil Kozar
none Details | Review
Patch porting teletextdec to 1.0, after review (38.55 KB, patch)
2014-08-04 21:19 UTC, Daniel Kamil Kozar
none Details | Review
Patch porting the teletext decoder to 1.0, with header updates (41.72 KB, patch)
2015-09-24 23:12 UTC, Daniel Kamil Kozar
none Details | Review
Patch porting the teletext decoder to 1.0 (38.50 KB, patch)
2015-09-26 09:27 UTC, Daniel Kamil Kozar
committed Details | Review

Description Daniel Kamil Kozar 2014-07-27 13:36:44 UTC
Created attachment 281814 [details] [review]
Patch porting teletextdec to 1.0

The attachment contains a patch with a 1.0 port of the teletextdec plugin. I've been able to successfully build and run this plugin with the latest GStreamer from master.

Other changes include :
 * The element no longer sends a "dummy buffer for preroll", as it relied on the src_setcaps function being called before pushing the first buffer. This is now fixed.
 * teletext.c has been deleted, as it did not serve any particular purpose.
 * GQueue and GMutex has been replaced with GstAtomicQueue.
 * The output function is now selected via a pointer-to-function instead of an enum.
 * Fix some warnings related to unused arguments and/or signed-unsigned comparison.
 * VBI structures (PES demuxer and HTML export module) are allocated only when needed, instead of always (in case of demuxer) or on every push (in case of exporter).

This element can also accept VBI teletext data packed inside MPEG PES packets. This capability is exposed as "video/mpeg,mpegversion=2,systemstream=TRUE", which is obviously wrong, but I was not sure how to say "teletext packed inside a PES" in caps-speak. Suggestions welcome.
Comment 1 Daniel Kamil Kozar 2014-07-27 15:47:45 UTC
Created attachment 281822 [details] [review]
Change caps to application/x-teletext, don't include gstatomicqueue.h

The caps of teletextdec's sinkpad should be application/x-teletext, which allows to link it with tsdemux. Also, including gstatomicqueue.h is not necessary anymore.
Comment 2 Edward Hervey 2014-07-28 09:02:06 UTC
Review of attachment 281814 [details] [review]:

::: ext/teletextdec/gstteletextdec.c
@@ +126,3 @@
+    GST_STATIC_CAPS ("video/mpeg,mpegversion=2,systemstream=TRUE;"
+        "private/teletext;")

fwiw, supporting mpeg systemstream isn't that useful and not in sync with how gstreamer elements work (do one thing, do it well).

That would remove quite a bit of code also.

@@ +134,3 @@
+        "text/x-raw, format={utf-8,pango-markup} ; text/html ;")

While RGBA and text/pango output make sense ... text/html makes no sense whatsoever.

How would one use it ?

That would remove yet some more code.

@@ +311,3 @@
       gst_teletextdec_event_handler, teletext);
+  teletext->queue = gst_atomic_queue_new (16);

Is there any real functional reason to switch from cond/gqueue to atomic queue ? Would be nice to keep the port patch as concise as possible.

@@ +418,3 @@
+      if (NULL == teletext->export_func) {
+        /* save the segment event and send it after sending caps. */
+        teletext->segment = event;

You should check if a segment event is already present (and not pushed), and if so remove the previous one.

@@ +487,3 @@
+  ret =
+      GST_ELEMENT_CLASS (gst_teletextdec_parent_class)->change_state (element,

fwiw, you can avoid doing those changes by just using a #define gst_teletext_parent_class parent_class at the top of the file

@@ +635,2 @@
   GstTeletextDec *teletext = GST_TELETEXTDEC (user_data);
+  (void) dx;


@@ +685,3 @@
+static void
+gst_teletextdec_negotiate_caps (GstTeletextDec * teletext,

There is something missing in this, which is to do an ALLOCATION query to figure out if downstream can provide a GstBufferPool, and if so, use buffers from that pool instead of creating our own (assuming the underlying library can be provided buffers in which to write)

@@ +757,2 @@
   GstFlowReturn ret = GST_FLOW_OK;
+  (void) pad;


@@ +966,3 @@
+  GstBuffer *lbuf;
+  GstMapInfo buf_map;
+  (void) teletext;

Comment 3 Sebastian Dröge (slomo) 2014-08-01 12:10:32 UTC
*** Bug 734002 has been marked as a duplicate of this bug. ***
Comment 4 Sebastian Dröge (slomo) 2014-08-01 12:16:57 UTC
(In reply to comment #2)

> @@ +966,3 @@
> +  GstBuffer *lbuf;
> +  GstMapInfo buf_map;
> +  (void) teletext;
> ??

These are to silence compiler warnings if the function parameters are unused. Alternatively one can put G_GNUC_UNUSED to the parameters. But we generally don't care about this kind of compiler warning, so these things should just be removed :)

Also our default compiler warning parameters don't report this.
Comment 5 Daniel Kamil Kozar 2014-08-04 21:19:27 UTC
Created attachment 282482 [details] [review]
Patch porting teletextdec to 1.0, after review

The patch porting teletextdec to 1.0, with the review comments addressed.
Comment 6 Daniel Kamil Kozar 2014-08-04 21:44:09 UTC
Some extra comments about the new patch :
 * Asking the peer pad for a buffer pool is done only if RGBA output was selected. This is mainly due to the fact that the text modes send out variable-sized buffers, making it impossible to use a pool. Also, text buffers are much smaller than RGBA buffers.
 * Some changes were needed regarding the usage of GMutex due to g_mutex_new() being deprecated in Glib 2.32 and replaced with g_mutex_init(). As a result of that, the GstTeletextDec structure contains an embedded instance of GMutex instead of a pointer.
 * Fake usages of unused parameters have been removed. I must've gotten overly zealous with the -Wextra.
 * All the code related to reading PES streams and HTML output has been removed.
Comment 7 Daniel Kamil Kozar 2014-09-15 09:23:56 UTC
What's the final decision about this patch? Should I change or add anything else?
Comment 8 Evgeny Kolesnikov 2015-07-05 13:15:18 UTC
Is there any movement on this port?
Comment 9 Thomas Löwe 2015-09-21 11:53:21 UTC
Tried it on Debian and works fine, but the clock should refreshing permanently and not only when a page update arrives.

Maybe in the eventhandler for VBI_EVENT_TTX_PAGE the ev->ev.ttx_page.clock_update could be used to get the current time from the last received page and "mixed" this time with the currently displayed page?
Comment 10 Daniel Kamil Kozar 2015-09-24 23:11:51 UTC
I've tried to find a solution to this before. However, there is one very significant problem : there is no way of knowing where the clock appears. The teletext spec (ETS 300 706) does not place any restrictions on the placement of the clock, and different broadcasters put it in varying regions in the first line of the page. All the transmissions in Poland have the clock on the right side, but I've seen streams where the clock was placed on the left, or in the centre of the header.
The most reasonable compromise, in my opinion, is to simply have the header refreshed with every page that the decoder receives, while maintaining the body of the page unmodified. However, this makes it look like the decoder is still "searching" for the page, since the page number in the header changes as well. Same as with the clock, the position of the page number is also up for the broadcaster to choose.
However, analogue TVs were somehow capable of keeping the page number still after decoding the requested page, while still updating the clock. I would really like to know how this was done, as there is no data in the teletext packets that would enable the decoder to update only the requested region of the page header. This video shows this behaviour quite well :
Anyhow, the current patch is based on the current git master, and updates the page header with every page update. Let me know what you think.
Comment 11 Daniel Kamil Kozar 2015-09-24 23:12:59 UTC
Created attachment 312092 [details] [review]
Patch porting the teletext decoder to 1.0, with header updates
Comment 12 Thomas Löwe 2015-09-25 06:28:14 UTC
Thanks for your response Daniel. I wasn't aware that the clock can be at any position.

The current approach is very irritating because of the refreshing page numbers. Another problem is that the colors differ on some pages (mixed teletext levels?).

Maybe it's better to make this in a seperate patch to keep the port clean, because I have 2 patches ( and submitted which are based on this one and I fear the changes will break it.

Maybe you are interested for looking at these too and improve it? For example I didn't know how to force an instant page update and I'm not sure if the page_update() function is the correct way. It works only when setting the sink to false.

Looks like nobody else is interested in teletext for gstreamer... :(
Comment 13 Daniel Kamil Kozar 2015-09-26 09:27:43 UTC
Created attachment 312183 [details] [review]
Patch porting the teletext decoder to 1.0
Comment 14 Daniel Kamil Kozar 2015-09-26 09:28:45 UTC
The patch I just added contains the (hopefully) final version of the actual port. I'll create another bug report for the header updates once I finish it.
Comment 15 Thomas Löwe 2015-12-06 08:47:58 UTC
Dear gstreamer maintainers,

could somebody please explain why there is absolutely no progress since over a year!?
Comment 16 Edward Hervey 2015-12-06 09:45:22 UTC

  Sorry for the delay, this was on my backlist of things to review/check. It looked clean enough and worked good enough (with hardcoded pipeline). So let's make it happen :)

commit 717f9a287d2bac1f343b072616ca3e4ae9850426
Author: Daniel Kamil Kozar <>
Date:   Thu Sep 24 17:40:02 2015 +0200

    port teletextdec to 1.0
Comment 17 Thomas Löwe 2015-12-06 09:51:01 UTC
Hi Edward,

thank you very much!

Could you please take a look at

Comment 18 Nicolas Dufresne (ndufresne) 2015-12-06 14:43:38 UTC
(Just a note, it's important for submitters to ping to get attention. Submitters can also be to blame for long delays.)
Comment 19 Thomas Löwe 2016-04-18 06:32:48 UTC
> "it's important for submitters to ping to get attention"

Well, how often submitters should ping?

Look at the 2 patches above: no reaction (one of them was submitted in 2011).

That's really frustrating, because these functions are required to make a teletext client user friendly.

Sorry for off topic...