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 728364 - mpegts: add service list, stuffing and bouquet name descriptors
mpegts: add service list, stuffing and bouquet name descriptors
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.3.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-16 17:53 UTC by Stefan Ringel
Modified: 2014-05-26 09:57 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
add service list descriptor (3.86 KB, patch)
2014-04-16 17:53 UTC, Stefan Ringel 		needs-work Details | Review
add stuffing descriptor (3.00 KB, patch)
2014-04-16 17:54 UTC, Stefan Ringel 		accepted-commit_now Details | Review
add bouquet name descriptor (2.90 KB, patch)
2014-04-16 17:54 UTC, Stefan Ringel 		needs-work Details | Review
add service list descriptor (3.85 KB, patch)
2014-04-16 18:30 UTC, Stefan Ringel 		accepted-commit_now Details | Review
add stuffing descriptor (3.00 KB, patch)
2014-04-16 18:31 UTC, Stefan Ringel 		needs-work Details | Review
add bouquet name descriptor (2.90 KB, patch)
2014-04-16 18:31 UTC, Stefan Ringel 		accepted-commit_now Details | Review
add service list descriptor (3.85 KB, patch)
2014-04-17 12:57 UTC, Stefan Ringel 		committed Details | Review
add stuffing descriptor (3.00 KB, patch)
2014-04-17 12:57 UTC, Stefan Ringel 		committed Details | Review
add bouquet name descriptor (2.90 KB, patch)
2014-04-17 12:57 UTC, Stefan Ringel 		committed Details | Review

Description Stefan Ringel 2014-04-16 17:53:45 UTC 
Created attachment 274503 [details] [review]
add service list descriptor

mpegts: add service list, stuffing and bouquet name descriptors
Comment 1 Stefan Ringel 2014-04-16 17:54:05 UTC 
Created attachment 274504 [details] [review]
add stuffing descriptor
Comment 2 Stefan Ringel 2014-04-16 17:54:28 UTC 
Created attachment 274505 [details] [review]
add bouquet name descriptor
Comment 3 Tim-Philipp Müller 2014-04-16 18:10:10 UTC 
Comment on attachment 274503 [details] [review]
add service list descriptor

Please don't use

for (guint i = 0; ...)

But

guint i;

for (i= 0; ....)

Thanks.
Comment 4 Sebastian Dröge (slomo) 2014-04-16 18:19:35 UTC 
Review of attachment 274505 [details] [review]:

::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
@@ +410,3 @@
+  data = (guint8 *) descriptor->data + 2;
+
+  *bouquet_name = get_encoding_and_convert ((const gchar *) data + 1, *data);

You are reading two bytes here but the size check is only for one byte
Comment 5 Stefan Ringel 2014-04-16 18:30:47 UTC 
Created attachment 274511 [details] [review]
add service list descriptor
Comment 6 Stefan Ringel 2014-04-16 18:31:10 UTC 
Created attachment 274512 [details] [review]
add stuffing descriptor
Comment 7 Stefan Ringel 2014-04-16 18:31:30 UTC 
Created attachment 274513 [details] [review]
add bouquet name descriptor
Comment 8 Sebastian Dröge (slomo) 2014-04-17 08:00:33 UTC 
Review of attachment 274512 [details] [review]:

::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
@@ +177,3 @@
+
+  g_return_val_if_fail (descriptor != NULL && stuffing_bytes != NULL, FALSE);
+  __common_desc_checks (descriptor, GST_MTS_DESC_DVB_STUFFING, 2, FALSE);

Why 2? Even 0 would be ok, no?
Comment 9 Sebastian Dröge (slomo) 2014-04-17 08:02:56 UTC 
Please also remove the "signed-off" line from the commit message, and instead put the URL to this bug report there (in all patches) :)
Comment 10 Stefan Ringel 2014-04-17 12:57:05 UTC 
Created attachment 274595 [details] [review]
add service list descriptor
Comment 11 Stefan Ringel 2014-04-17 12:57:27 UTC 
Created attachment 274596 [details] [review]
add stuffing descriptor
Comment 12 Stefan Ringel 2014-04-17 12:57:47 UTC 
Created attachment 274597 [details] [review]
add bouquet name descriptor
Comment 13 Edward Hervey 2014-05-26 09:56:44 UTC 
Pushed.

There was bug in the bouquet name descriptor parsing (it's not length-prefixed)

In the future it would be nice if you could include support for the tsparser example when you add descriptor handling. Allows everyone to be on the same page and would avoid discovering such issues.

commit 8d71ec9f389337be19266b9ec018e09b677115ab
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Mon May 26 11:55:31 2014 +0200

    examples: Add support for DVB Bouquet Name parsing

commit 42c061b0e739cd4956862dc4a876cf55cebd72ac
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Mon May 26 11:54:50 2014 +0200

    mpegts: Fix Bouquet Name parsing
    
    the field is not length prefixed

commit 2240630b041b5f84c80c1d28dc0530160597e825
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Mon May 26 11:42:46 2014 +0200

    examples: Add support for DVB Service List descriptor

commit bc7cf1520c4b303c9bbc4fb99624cda9a432b417
Author: Stefan Ringel <linuxtv@stefanringel.de>
Date:   Thu Apr 17 14:56:23 2014 +0200

    mpegts: add bouquet name descriptor
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728364

commit 985d19deaa59514227f8f655a07a5651b31e5bfe
Author: Stefan Ringel <linuxtv@stefanringel.de>
Date:   Thu Apr 17 14:55:29 2014 +0200

    mpegts: add stuffing descriptor
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728364

commit 570f78df799bfeedfaf029e29da22db2dc49959a
Author: Stefan Ringel <linuxtv@stefanringel.de>
Date:   Thu Apr 17 14:54:28 2014 +0200

    mpegts: add service list descriptor
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728364