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 727560 - mpegts: add frequency list, scrambling and dvb ca indentifier descriptor
mpegts: add frequency list, scrambling and dvb ca indentifier descriptor
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-03 18:20 UTC by Stefan Ringel
Modified: 2014-04-16 07:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add frequency list descriptor (4.95 KB, patch)
2014-04-03 18:20 UTC, Stefan Ringel
needs-work Details | Review
scrambling descriptor (4.19 KB, patch)
2014-04-03 18:20 UTC, Stefan Ringel
accepted-commit_now Details | Review
add dvb ca indentifier descriptor (3.32 KB, patch)
2014-04-03 18:21 UTC, Stefan Ringel
needs-work Details | Review
add frequency list descriptor (4.96 KB, patch)
2014-04-04 12:34 UTC, Stefan Ringel
accepted-commit_now Details | Review
add scrambling descriptor (4.19 KB, patch)
2014-04-04 12:35 UTC, Stefan Ringel
accepted-commit_now Details | Review
add dvb ca indentifier descriptor (3.84 KB, patch)
2014-04-04 12:36 UTC, Stefan Ringel
accepted-commit_now Details | Review
add frequency list descriptor (4.87 KB, patch)
2014-04-09 08:15 UTC, Stefan Ringel
accepted-commit_now Details | Review
add scrambling descriptor (4.19 KB, patch)
2014-04-09 08:16 UTC, Stefan Ringel
accepted-commit_now Details | Review
add dvb ca indentifier descriptor (3.35 KB, patch)
2014-04-09 08:16 UTC, Stefan Ringel
accepted-commit_now Details | Review
add frequency list descriptor (4.87 KB, patch)
2014-04-10 14:08 UTC, Stefan Ringel
committed Details | Review
add scrambling descriptor (4.19 KB, patch)
2014-04-10 14:09 UTC, Stefan Ringel
committed Details | Review
add dvb ca indentifier descriptor (3.35 KB, patch)
2014-04-10 14:09 UTC, Stefan Ringel
committed Details | Review

Description Stefan Ringel 2014-04-03 18:20:23 UTC
Created attachment 273543 [details] [review]
add frequency list descriptor

mpegts: add frequency list, scrambling and dvb ca indentifier descriptor
Comment 1 Stefan Ringel 2014-04-03 18:20:48 UTC
Created attachment 273544 [details] [review]
scrambling descriptor
Comment 2 Stefan Ringel 2014-04-03 18:21:16 UTC
Created attachment 273545 [details] [review]
add dvb ca indentifier descriptor
Comment 3 Sebastian Dröge (slomo) 2014-04-04 07:51:58 UTC
Review of attachment 273543 [details] [review]:

::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
@@ +1254,3 @@
+
+  if (type == 1) {
+    // satellite

Don't use C99 comments

@@ +1265,3 @@
+  len = descriptor->length - 1;
+
+  for (guint8 i = 0; i < len; i += 4) {

If I'm not missing anything this could read over the end of the byte array. It should be i < len - 3
Comment 4 Sebastian Dröge (slomo) 2014-04-04 07:53:01 UTC
Review of attachment 273545 [details] [review]:

::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
@@ +915,3 @@
+  __common_desc_checks (descriptor, GST_MTS_DESC_DVB_CA_IDENTIFIER, 2, FALSE);
+
+  data = (guint8 *) descriptor->data + 2;

What's in the first two bytes?

@@ +919,3 @@
+  *list = g_array_new (FALSE, FALSE, sizeof (guint16));
+
+  for (guint i = 0; i < descriptor->length; i += 2) {

Same as in the other one, i < descriptor->length - 1. Probably more of these things in the already existing code.
Comment 5 Stefan Ringel 2014-04-04 09:22:32 UTC
(In reply to comment #3)
> Review of attachment 273543 [details] [review]:
> 
> ::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
> @@ +1254,3 @@
> +
> +  if (type == 1) {
> +    // satellite
> 
> Don't use C99 comments
o.k.
> 
> @@ +1265,3 @@
> +  len = descriptor->length - 1;
> +
> +  for (guint8 i = 0; i < len; i += 4) {
> 
> If I'm not missing anything this could read over the end of the byte array. It
> should be i < len - 3
o.k.
Comment 6 Stefan Ringel 2014-04-04 09:28:24 UTC
(In reply to comment #4)
> Review of attachment 273545 [details] [review]:
> 
> ::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
> @@ +915,3 @@
> +  __common_desc_checks (descriptor, GST_MTS_DESC_DVB_CA_IDENTIFIER, 2, FALSE);
> +
> +  data = (guint8 *) descriptor->data + 2;
> 
> What's in the first two bytes?
The first bytes are always the tag and the length (be parsed in descriptor class).
> 
> @@ +919,3 @@
> +  *list = g_array_new (FALSE, FALSE, sizeof (guint16));
> +
> +  for (guint i = 0; i < descriptor->length; i += 2) {
> 
> Same as in the other one, i < descriptor->length - 1. Probably more of these
> things in the already existing code.

o.k. If you those say that it also in existing code then thanks. But not all have constant length of bytes. Is while {} do in this case better?
Comment 7 Stefan Ringel 2014-04-04 12:34:54 UTC
Created attachment 273586 [details] [review]
add frequency list descriptor
Comment 8 Stefan Ringel 2014-04-04 12:35:33 UTC
Created attachment 273587 [details] [review]
add scrambling descriptor
Comment 9 Stefan Ringel 2014-04-04 12:36:04 UTC
Created attachment 273588 [details] [review]
add dvb ca indentifier descriptor
Comment 10 Sebastian Dröge (slomo) 2014-04-08 09:37:27 UTC
Comment on attachment 273586 [details] [review]
add frequency list descriptor

All look good but depend on other patches apparently as they don't apply cleanly right now against git master.
Comment 11 Stefan Ringel 2014-04-09 08:15:45 UTC
Created attachment 273863 [details] [review]
add frequency list descriptor
Comment 12 Stefan Ringel 2014-04-09 08:16:11 UTC
Created attachment 273864 [details] [review]
add scrambling descriptor
Comment 13 Stefan Ringel 2014-04-09 08:16:46 UTC
Created attachment 273865 [details] [review]
add dvb ca indentifier descriptor
Comment 14 Sebastian Dröge (slomo) 2014-04-10 07:14:51 UTC
Will merge once the other patches are merged, without them these don't apply cleanly.
Comment 15 Stefan Ringel 2014-04-10 14:08:58 UTC
Created attachment 273995 [details] [review]
add frequency list descriptor
Comment 16 Stefan Ringel 2014-04-10 14:09:29 UTC
Created attachment 273996 [details] [review]
add scrambling descriptor
Comment 17 Stefan Ringel 2014-04-10 14:09:49 UTC
Created attachment 273997 [details] [review]
add dvb ca indentifier descriptor
Comment 18 Sebastian Dröge (slomo) 2014-04-16 07:23:56 UTC
commit eeaf6e481127bdba4f2f0fd4233e00400f69adaa
Author: Stefan Ringel <linuxtv@stefanringel.de>
Date:   Thu Apr 10 16:04:21 2014 +0200

    mpegts: add dvb ca identifier descriptor
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727560

commit 0c773b8cddbde4548c6d657c47de0fab82ba9e6f
Author: Stefan Ringel <linuxtv@stefanringel.de>
Date:   Thu Apr 10 16:03:07 2014 +0200

    mpegts: add scrambling descriptor
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727560

commit e535967ee99fcdbd8936ab706f400b8023301d32
Author: Stefan Ringel <linuxtv@stefanringel.de>
Date:   Thu Apr 10 16:02:09 2014 +0200

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