GNOME Bugzilla – Bug 727560
mpegts: add frequency list, scrambling and dvb ca indentifier descriptor
Last modified: 2014-04-16 07:24:04 UTC
Created attachment 273543 [details] [review] add frequency list descriptor mpegts: add frequency list, scrambling and dvb ca indentifier descriptor
Created attachment 273544 [details] [review] scrambling descriptor
Created attachment 273545 [details] [review] add dvb ca indentifier descriptor
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
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.
(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.
(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?
Created attachment 273586 [details] [review] add frequency list descriptor
Created attachment 273587 [details] [review] add scrambling descriptor
Created attachment 273588 [details] [review] add dvb ca indentifier descriptor
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.
Created attachment 273863 [details] [review] add frequency list descriptor
Created attachment 273864 [details] [review] add scrambling descriptor
Created attachment 273865 [details] [review] add dvb ca indentifier descriptor
Will merge once the other patches are merged, without them these don't apply cleanly.
Created attachment 273995 [details] [review] add frequency list descriptor
Created attachment 273996 [details] [review] add scrambling descriptor
Created attachment 273997 [details] [review] add dvb ca indentifier descriptor
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