[Bug 671136] mpegtsmux: add support for SDT and NIT tables for DVB-S/DVB-T

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=671136
  GStreamer | gst-plugins-bad | git

Edward Hervey <bilboed> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #257626|none                        |needs-work
             status|                            |

--- Comment #7 from Edward Hervey <bilboed at bilboed.com> 2013-10-27 11:26:52 UTC ---
Review of attachment 257626:
 --> (https://bugzilla.gnome.org/review?bug=671136&attachment=257626)

Definitely going in the right direction.

Some stuff should be split in a separate commit, and some code should be made
more re-usable.

Was wondering if we want to separate the section_from_* from the actual
serialization, but then realized that since it's the app creating those
sections, it does indeed *want* to have those serialized.

::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
@@ +95,3 @@
+  g_return_val_if_fail (strlen (name) <= 256, NULL);
+
+  descriptor = g_slice_new0 (GstMpegTsDescriptor);

For the sake of code reduction (and avoiding bugs), we should have a generic
internal function to do all the following:
* Create slice of the proper size
* Initialize common variables (tag, length, data)
* Allocate output data
* Set initial descriptor data values (tag, length)

@@ +108,3 @@
+  *data++ = descriptor->length;
+
+  memcpy (data, name, strlen (name));

This only supports the ISO 6937 character set.

We need an opposite function to get_encoding_and_convert

::: gst-libs/gst/mpegts/gst-dvb-section.c
@@ +490,3 @@
   end = data + section->section_length;

+  /* Set network id, and skip the rest of what is already parsed */

Put in a separate commit and just re-use the generic subtable_extension field
instead of re-parsing it.

@@ +653,3 @@
+/**
+ * gst_mpegts_section_from_nit:
+ * @nit: a #GstMpegTsNIT to create the #GstMpegTsSection from

what's the transfer method ? none, full ?

@@ +705,3 @@
+  g_return_val_if_fail (size <= 1024, NULL);
+
+  data = g_malloc (size);

A common internal method for creating sections would be nice (for the same
reason as descriptors)

@@ +731,3 @@
+  *data++ = 0xC1;
+
+  /* section_number                   - 8  bit uimsbf */

How will we handle multiple sections ?

@@ +741,3 @@
+  data += 2;
+
+  if (nit->descriptors) {

Common internal function for serializing array of descriptors

@@ +795,3 @@
+
+  /* We have a parsed GstMpegTsNIT */
+  section->cached_parsed = (gpointer) _gst_mpegts_nit_copy (nit);

Shouldn't we just transfer ownership to the section ? Do we plan of re-using a
NIT accross multiple sections ?

::: gst-libs/gst/mpegts/gst-dvb-section.h
@@ +155,3 @@
 {
   gboolean   actual_network;
+  guint16    network_id;

Separate commit

::: gst-libs/gst/mpegts/gstmpegtssection.c
@@ +795,3 @@
+
+/**
+ * gst_mpegts_section_packetize:

_private_section_packetize ?

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.