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 671136 - mpegtsmux: add support for SDT and NIT tables for DVB-S/DVB-T
mpegtsmux: add support for SDT and NIT tables for DVB-S/DVB-T
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: 721682 721685
 
 
Reported: 2012-03-01 15:14 UTC by Sam
Modified: 2014-02-06 15:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mpeg-ts changes (3.38 KB, patch)
2013-10-16 10:26 UTC, Jesper Larsen
none Details | Review
mpeg-ts changes v2 (16.21 KB, patch)
2013-10-18 09:23 UTC, Jesper Larsen
needs-work Details | Review
Add support for two-way iconv conversions (3.83 KB, patch)
2013-10-30 10:02 UTC, Jesper Larsen
none Details | Review
Use big endian UCS-2 coding (1.20 KB, patch)
2013-10-30 10:03 UTC, Jesper Larsen
none Details | Review
Add function to encode UTF8 strings (5.55 KB, patch)
2013-10-30 10:03 UTC, Jesper Larsen
none Details | Review
Add network_id to GstMpegTsNIT (1.32 KB, patch)
2013-10-30 10:04 UTC, Jesper Larsen
none Details | Review
Add init functions for descriptor/section (5.66 KB, patch)
2013-10-30 10:04 UTC, Jesper Larsen
none Details | Review
Add functions to packetize section (5.32 KB, patch)
2013-10-30 10:05 UTC, Jesper Larsen
none Details | Review
Add support for creating a NIT section (6.15 KB, patch)
2013-10-30 10:05 UTC, Jesper Larsen
none Details | Review
network name descriptor creation (2.46 KB, patch)
2013-10-30 10:05 UTC, Jesper Larsen
none Details | Review
mpegts: Add functions to send sections as events (5.38 KB, patch)
2013-10-31 14:22 UTC, Jesper Larsen
none Details | Review
tsdemux: Add known PIDs for NIT, SDT, EIT, RST (1.12 KB, patch)
2013-10-31 14:22 UTC, Jesper Larsen
none Details | Review
examples: ts-parser: Add table_id_name function (2.76 KB, patch)
2013-10-31 14:23 UTC, Jesper Larsen
none Details | Review
mpegtsmux: Add support for muxing NIT tables (15.28 KB, patch)
2013-10-31 14:23 UTC, Jesper Larsen
none Details | Review
Test case that will write a NIT to the stream (1.85 KB, text/x-csrc)
2013-10-31 14:28 UTC, Jesper Larsen
  Details

Description Sam 2012-03-01 15:14:35 UTC
Add support for 2 more tables SDT and NIT this is a optional feature but can
help significantly when trying DVB-S or DVB-T encoding.
Comment 1 Edward Hervey 2013-07-17 06:35:33 UTC
This could use the new mpeg-ts library (Create SDT/NIT and set them on element)
Comment 2 Jesper Larsen 2013-10-14 09:19:53 UTC
I'm currently working on the implementation of these tables.

My preliminary thoughts are to make a property for the mpegtsmux element, that will take an array of GstMpegTsSection. The sections can be created in the user application by using the mpeg-ts function gst_mpegts_section_new

The sections will be inserted in the stream at the same time as PMT/PAT.

Suggestions to a different approach are welcome.
Comment 3 Jesper Larsen 2013-10-16 10:26:12 UTC
Created attachment 257410 [details] [review]
mpeg-ts changes

I have a basic implementation working now. I will wait for the refactoring in https://bugzilla.gnome.org/show_bug.cgi?id=709826 in order to adapt the changes I have in gst/mpegtsmux.

However, I would like to discuss changes to the mpeg-ts lib.

What is the general intention for GstMpegTsSection? Would it ideally replace the current TsMuxSection?

The mpeg-ts lib is mostly tuned for parsing at the moment. The patch I attached is the changes I have made to get my basic implementation working. It includes a function used to synchronize the public part of GstMpegTsSection with the data field. Basically it is the reverse of the gst_mpegts_section_new function.

I added a small set_version function to illustrate how updates of the section could happen. In this approach it would make sense to mark the version_number as private instead of public though. I'm not really fancying that.

The muxer in my implementation gets the data through the gst_mpegts_section_get_buffer function, which just wraps the sections data field in a GstBuffer.

Is any of this in line with the thoughts about the mpeg-ts lib?
Comment 4 Edward Hervey 2013-10-16 12:29:15 UTC
(In reply to comment #3)
> Created an attachment (id=257410) [details] [review]
> mpeg-ts changes
> 
> I have a basic implementation working now. I will wait for the refactoring in
> https://bugzilla.gnome.org/show_bug.cgi?id=709826 in order to adapt the changes
> I have in gst/mpegtsmux.
> 
> However, I would like to discuss changes to the mpeg-ts lib.
> 
> What is the general intention for GstMpegTsSection? Would it ideally replace
> the current TsMuxSection?

  It was in the back of my mind when designing the mpeg-ts lib (to be honest, until a few days ago I hadn't looked that much in depth at mpegtsmux).

> 
> The mpeg-ts lib is mostly tuned for parsing at the moment. The patch I attached
> is the changes I have made to get my basic implementation working. It includes
> a function used to synchronize the public part of GstMpegTsSection with the
> data field. Basically it is the reverse of the gst_mpegts_section_new function.
> 
> I added a small set_version function to illustrate how updates of the section
> could happen. In this approach it would make sense to mark the version_number
> as private instead of public though. I'm not really fancying that.
> 
> The muxer in my implementation gets the data through the
> gst_mpegts_section_get_buffer function, which just wraps the sections data
> field in a GstBuffer.
> 
> Is any of this in line with the thoughts about the mpeg-ts lib?


One potential idea would be to be able to create the opposite of the gst_mpegts_section_get_* and have gst_mpegts_section_from_*

Ex:
GstMpegTSSection *gst_mpegts_section_from_nit(GstMpegTsNIT *nit);

That would store the argument in GstMpegTSSection's parsed_data (private).

You could then set whatever public fields you want on the section (say the version) and then call a generic function to serialize it to a binary section data (which you can then inject in a muxer).

guint8 *gst_mpegts_section_packetize(GstMpegTSSection *section, gsize *output_size);
Comment 5 Jesper Larsen 2013-10-16 13:26:10 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Created an attachment (id=257410) [details] [review] [details] [review]
> > mpeg-ts changes
> > 
> > I have a basic implementation working now. I will wait for the refactoring in
> > https://bugzilla.gnome.org/show_bug.cgi?id=709826 in order to adapt the changes
> > I have in gst/mpegtsmux.
> > 
> > However, I would like to discuss changes to the mpeg-ts lib.
> > 
> > What is the general intention for GstMpegTsSection? Would it ideally replace
> > the current TsMuxSection?
> 
>   It was in the back of my mind when designing the mpeg-ts lib (to be honest,
> until a few days ago I hadn't looked that much in depth at mpegtsmux).

Much of the data is redundant, but the GstMpegTsSection lacks data about the packet (continuity_counter etc.). 

> 
> > 
> > The mpeg-ts lib is mostly tuned for parsing at the moment. The patch I attached
> > is the changes I have made to get my basic implementation working. It includes
> > a function used to synchronize the public part of GstMpegTsSection with the
> > data field. Basically it is the reverse of the gst_mpegts_section_new function.
> > 
> > I added a small set_version function to illustrate how updates of the section
> > could happen. In this approach it would make sense to mark the version_number
> > as private instead of public though. I'm not really fancying that.
> > 
> > The muxer in my implementation gets the data through the
> > gst_mpegts_section_get_buffer function, which just wraps the sections data
> > field in a GstBuffer.
> > 
> > Is any of this in line with the thoughts about the mpeg-ts lib?
> 
> 
> One potential idea would be to be able to create the opposite of the
> gst_mpegts_section_get_* and have gst_mpegts_section_from_*
> 
> Ex:
> GstMpegTSSection *gst_mpegts_section_from_nit(GstMpegTsNIT *nit);
> 
> That would store the argument in GstMpegTSSection's parsed_data (private).

So gst_mpegts_section_from_nit would update the public fields, and set the parsed_data to GstMpegTsNIT* ?

> 
> You could then set whatever public fields you want on the section (say the
> version) and then call a generic function to serialize it to a binary section
> data (which you can then inject in a muxer).

This will enable us to serialize the generic section header (first 8 bytes). We then introduce a _serialize function to serialize the table it self.

Ex.
static guint8 *_serialize_nit (GstMpegTsNIT *nit, gsize * output_size);

>
> guint8 *gst_mpegts_section_packetize(GstMpegTSSection *section, gsize
> *output_size);

This function will then

1. serialize the section header
2. serialize the section content based on the section type. If section type is unknown pass bytes from the data field (After the header).
3. update CRC if applicable
Comment 6 Jesper Larsen 2013-10-18 09:23:34 UTC
Created attachment 257626 [details] [review]
mpeg-ts changes v2

A new suggestion to the changes in mpeg-ts lib.

The changes supports the NIT table. I have tested it by creating a NIT section with a network name descriptor.
The flow is outlined below

- A GstMpegTsNIT is created using the new lib function
  GstMpegTsNIT *gst_mpegts_section_new_nit (void);

- Top level fields (network_id, and actual_network) is set

- A GstMpegTsDescriptor is created using new lib function
  GstMpegTsDescriptor *gst_mpegts_descriptor_from_dvb_network_name (const gchar *name)

- The descriptor is added to the GstMpegTsNIT descriptor field

- A GstMpegTsSection is created by the new lib function
  GstMpegTsSection *gst_mpegts_section_from_nit (GstMpegTsNIT *nit);

- The GstMpegTsSection is injected into the muxer using the new lib function
  guint8 *gst_mpegts_section_packetize (GstMpegTsSection *section, gsize *output_size);

I don't like the *_new_nit* functions that well (guess they also would require a free function). Am I missing some GLib magic that can help us there?
Comment 7 Edward Hervey 2013-10-27 11:26:52 UTC
Review of attachment 257626 [details] [review]:

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 ?
Comment 8 Edward Hervey 2013-10-27 11:28:48 UTC
(In reply to comment #6)

> 
> I don't like the *_new_nit* functions that well (guess they also would require
> a free function). Am I missing some GLib magic that can help us there?

They already are registered GTypes (with free/copy functions).
Comment 9 Jesper Larsen 2013-10-28 10:27:31 UTC
(In reply to comment #7)
> Review of attachment 257626 [details] [review]:
> 
> @@ +731,3 @@
> +  *data++ = 0xC1;
> +
> +  /* section_number                   - 8  bit uimsbf */
> 
> How will we handle multiple sections ?

Excellent question. This seems to be something that should be handled outside the library?

With the addition of the section_packetizer I guess there is the possibility of making the user responsible of updating the public section_number/last_section_number fields in the GstMpegTsSection?

> 
> @@ +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 ?
> 

Good point. Lets make the transfer full.

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

This function needs to be public for use in i.e. muxer?
Comment 10 Edward Hervey 2013-10-28 10:49:22 UTC
(In reply to comment #9)
> > How will we handle multiple sections ?
> 
> Excellent question. This seems to be something that should be handled outside
> the library?
> 
> With the addition of the section_packetizer I guess there is the possibility of
> making the user responsible of updating the public
> section_number/last_section_number fields in the GstMpegTsSection?

  Or we could accept sections bigger than the limit (1024) and automatically create multiple sections (with the (last_)section_number fields correctly set) ? But then it would break the 1:1 relationship between parsed stuff and sections.

> 
> > 
> > ::: gst-libs/gst/mpegts/gstmpegtssection.c
> > @@ +795,3 @@
> > +
> > +/**
> > + * gst_mpegts_section_packetize:
> > 
> > _private_section_packetize ?
> 
> This function needs to be public for use in i.e. muxer?

Sorry I thought it was the support for private sections (i.e. undefined type).

malloc-ing and memcpy-ing once again seems overkill.

We should just create the final entire section once. So maybe we do need to delay the packetization of the section-specific data to that point in time.

Store a packetize vfunc in GstMpegTSSection and use that one when we really need the fully packetized value ?
Comment 11 Jesper Larsen 2013-10-28 11:15:17 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > > How will we handle multiple sections ?
> > 
> > Excellent question. This seems to be something that should be handled outside
> > the library?
> > 
> > With the addition of the section_packetizer I guess there is the possibility of
> > making the user responsible of updating the public
> > section_number/last_section_number fields in the GstMpegTsSection?
> 
>   Or we could accept sections bigger than the limit (1024) and automatically
> create multiple sections (with the (last_)section_number fields correctly set)
> ? But then it would break the 1:1 relationship between parsed stuff and
> sections.

That sounds like the ideal solution. We could introduce a wrapper layer that encapsulates all the sections. Meaning that we have i.e. GstMpegTsNITable which will contain an array of the GstMpegTsNITSection that makes up the entire NIT?

> 
> 
> malloc-ing and memcpy-ing once again seems overkill.
> 
> We should just create the final entire section once. So maybe we do need to
> delay the packetization of the section-specific data to that point in time.
> 
> Store a packetize vfunc in GstMpegTSSection and use that one when we really
> need the fully packetized value ?

I like that idea. We can make a generic internal function to serialize the common part of the section, and then based on the section type serialize the information in cached_parsed.

We just need to make sure that we don't need to serialize a section that has been created with gst_mpegts_section_new and hasn't been parsed.
Comment 12 Jesper Larsen 2013-10-28 13:17:47 UTC
(In reply to comment #7)
> Review of attachment 257626 [details] [review]:
> 
> @@ +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
> 

What does the opposite function of get_encoding_and_convert do?

The descriptor supports all of the character maps described in Annex A of EN 300 568, does it not?

I can see a problem with using the strlen function, but do we need more than a function to get the actual size in bytes of the string?

The user can supply a char array that contains character selection, and that should be included in the descriptor?
Comment 13 Edward Hervey 2013-10-28 16:44:04 UTC
For all functions that take user strings we should have two variants:
* default, which takes a UTF8 string and figures out how to convert that to the proper character map, including leading character selection
* alternate, where you do the string creation yourself and it will be stored as-is with the specified size

Maybe the default behaviour could have an optional "character set" argument if you want to make sure the strings get converted to that set.

Every "user-visible" strings in GStreamer are UTF-8 (since it allows supporting pretty much every character sets out there), we should therefore make sure the default behaviour also works that way.
But I can also understand some people might want to be absolutely specific about which character set to use.
Comment 14 Jesper Larsen 2013-10-30 10:02:41 UTC
Created attachment 258535 [details] [review]
Add support for two-way iconv conversions
Comment 15 Jesper Larsen 2013-10-30 10:03:36 UTC
Created attachment 258536 [details] [review]
Use big endian UCS-2 coding

Decoding of control codes in multi byte character maps requires big endian
Comment 16 Jesper Larsen 2013-10-30 10:03:55 UTC
Created attachment 258537 [details] [review]
Add function to encode UTF8 strings
Comment 17 Jesper Larsen 2013-10-30 10:04:14 UTC
Created attachment 258538 [details] [review]
Add network_id to GstMpegTsNIT
Comment 18 Jesper Larsen 2013-10-30 10:04:36 UTC
Created attachment 258539 [details] [review]
Add init functions for descriptor/section
Comment 19 Jesper Larsen 2013-10-30 10:05:03 UTC
Created attachment 258540 [details] [review]
Add functions to packetize section
Comment 20 Jesper Larsen 2013-10-30 10:05:25 UTC
Created attachment 258541 [details] [review]
Add support for creating a NIT section
Comment 21 Jesper Larsen 2013-10-30 10:05:42 UTC
Created attachment 258542 [details] [review]
network name descriptor creation
Comment 22 Jesper Larsen 2013-10-31 14:22:19 UTC
Created attachment 258646 [details] [review]
mpegts: Add functions to send sections as events
Comment 23 Jesper Larsen 2013-10-31 14:22:42 UTC
Created attachment 258647 [details] [review]
tsdemux: Add known PIDs for NIT, SDT, EIT, RST
Comment 24 Jesper Larsen 2013-10-31 14:23:10 UTC
Created attachment 258648 [details] [review]
examples: ts-parser: Add table_id_name function
Comment 25 Jesper Larsen 2013-10-31 14:23:39 UTC
Created attachment 258649 [details] [review]
mpegtsmux: Add support for muxing NIT tables
Comment 26 Jesper Larsen 2013-10-31 14:28:50 UTC
Created attachment 258651 [details]
Test case that will write a NIT to the stream

Wanted to give a fully functional test case. Applying the patches attached here will allow you to test a basic NIT functionality.

Compiling test case:

$ gcc nit-test.c `pkg-config --cflags --libs gstreamer-1.0 gstreamer-mpegts-1.0` -o nit-test

Takes a gst-launch pipeline argument i.e.

$ ./nit-test filesrc location=video.h264 ! h264parse ! mpegtsmux name=mux ! filesink location=nit-test-out.ts

A NIT containing a network name descriptor will be sent to an element named mux.
Comment 27 Edward Hervey 2013-11-15 14:48:44 UTC
(In reply to comment #15)
> Created an attachment (id=258536) [details] [review]
> Use big endian UCS-2 coding
> 
> Decoding of control codes in multi byte character maps requires big endian

Is that compatible with the previous behaviour ?

(In reply to comment #17)
> Created an attachment (id=258538) [details] [review]
> Add network_id to GstMpegTsNIT

No need to split the +3 and +5 :)
Comment 28 Edward Hervey 2013-11-15 14:57:26 UTC
Review of attachment 258540 [details] [review]:

::: gst-libs/gst/mpegts/gstmpegtssection.c
@@ +744,3 @@
+  *data++ = section->table_id;
+
+  /* section_syntax_indicator         - 1  bit

section_syntax_indicator should be set accordingly (1 if !short_section, and 0 if it is a short_section).
Comment 29 Edward Hervey 2013-11-15 15:00:49 UTC
Review of attachment 258541 [details] [review]:

::: gst-libs/gst/mpegts/gst-dvb-section.c
@@ +702,3 @@
+  start = section->data;
+
+  _packetize_common_section (section);

The 3 lines above could also go in the common part

@@ +749,3 @@
+  }
+
+  /* CRC                              - 32 bit rpchof */

Could maybe be done automatically by the caller of the packetize() method ?
Comment 30 Edward Hervey 2013-11-15 15:03:40 UTC
Review of attachment 258542 [details] [review]:

::: gst-libs/gst/mpegts/gst-dvb-descriptor.c
@@ +97,3 @@
+
+  converted_name = encoding_from_utf8 (name, &size);
+  g_return_val_if_fail (converted_name != NULL, NULL);

This needs to be a proper check/return (and not a g_return_val_if_fail())
Comment 31 Edward Hervey 2013-11-15 15:07:16 UTC
Review of attachment 258647 [details] [review]:

::: gst/mpegtsdemux/mpegtsbase.c
@@ +179,3 @@
   MPEGTS_BIT_SET (base->known_psi, 2);
   MPEGTS_BIT_SET (base->known_psi, 3);
+  /* NIT, SDT, EIT, RST (ETSI EN 300 468 v1.11.1) */

Not a big fan of enabling those by default in the demuxer (it's already done in the parser).
Comment 32 Edward Hervey 2013-11-15 15:17:42 UTC
Review of attachment 258649 [details] [review]:

::: gst/mpegtsmux/mpegtsmux.c
@@ +284,3 @@
+  g_object_class_install_property (G_OBJECT_CLASS (klass), ARG_SI_INTERVAL,
+      g_param_spec_uint ("si-interval", "SI interval",
+          "Set the interval (in ticks of the 90kHz clock) for writing out the Service"

Just wondering if it wouldn't be more user friendly to have that in GstClockTime

@@ +1713,3 @@
+    /* TODO: Implement other section types */
+    if (GST_MPEGTS_SECTION_TYPE (section) == GST_MPEGTS_SECTION_NIT)
+      tsmux_add_mpegts_si_section (mux->tsmux, section);

is the reference transfer properly handled here ?

@@ +1717,3 @@
+    return TRUE;
+  }
+

Don't forget to unref the event :)
Comment 33 Jesper Larsen 2013-11-18 08:30:50 UTC
(In reply to comment #27)
> (In reply to comment #15)
> > Created an attachment (id=258536) [details] [review] [details] [review]
> > Use big endian UCS-2 coding
> > 
> > Decoding of control codes in multi byte character maps requires big endian
> 
> Is that compatible with the previous behaviour ?
> 

No, but I suspect a small bug in the previous behaviour. The problem is only seen on strings with a multi byte character map and with the special control codes. I.e. 0xE08A for newline.

The control codes are parsed in gst-libs/gst/mpegts/gstmpegtsdescriptor.c:convert_to_utf8

while (*text != '\0') {                                                                                
  guint16 code = GST_READ_UINT16_BE (text);                                                            
  switch (code) {                                                                                      
    case 0xE086:         /* emphasis on */                                                             
    case 0xE087:         /* emphasis off */                                                            
      /* skip it */                                                                                    
      break;                                                                                           
    case 0xE08A:{                                                                                      
      pos[0] = 0x00;      /* 0x00 0x0A is a new line */                                                
      pos[1] = 0x0A;                                                                                   
      pos += 2;                                                                                        
      break;                                                                                           
    }                                                                                                  
    default:                                                                                           
      pos[0] = text[0];                                                                                
      pos[1] = text[1];                                                                                
      pos += 2;                                                                                        
      break;                                                                                           
    }                                                                                                    
    text += 2;              
}

This parsing is big endian specific.

The character maps _ICONV_ISO10646_UC2 and _ICONV_UCS_2BE should be compliant, but the old _ICONV_ISO10646_UC2 produces little endian strings on my setup.
Comment 34 Jesper Larsen 2013-11-18 08:36:52 UTC
(In reply to comment #32)
> Review of attachment 258649 [details] [review]:
> 
> ::: gst/mpegtsmux/mpegtsmux.c
> @@ +284,3 @@
> +  g_object_class_install_property (G_OBJECT_CLASS (klass), ARG_SI_INTERVAL,
> +      g_param_spec_uint ("si-interval", "SI interval",
> +          "Set the interval (in ticks of the 90kHz clock) for writing out the
> Service"
> 
> Just wondering if it wouldn't be more user friendly to have that in
> GstClockTime

Yes it would. However, the properties for the PAT and PMT intervals are given in ticks. They should all three be the same.

We can change the PAT/PMT properties as well?

> 
> @@ +1713,3 @@
> +    /* TODO: Implement other section types */
> +    if (GST_MPEGTS_SECTION_TYPE (section) == GST_MPEGTS_SECTION_NIT)
> +      tsmux_add_mpegts_si_section (mux->tsmux, section);
> 
> is the reference transfer properly handled here ?

The section reference is transferred to tsmux_add_mpegts_si_section, which does not unref the section. The section is inserted in a HashTable, and the reference is unreffed by the HashTable value free function.
Comment 35 Jesper Larsen 2013-11-20 11:49:49 UTC
The patch series has been updated and moved to github

https://github.com/larseje/gst-plugins-bad/pull/1
Comment 36 Eric 2014-01-06 14:49:52 UTC
Hi,
the mpegts.h is missing the G_BEGIN_DECLS resulting in linkage error when trying to use it from cpp.
Comment 37 Tim-Philipp Müller 2014-01-06 15:08:07 UTC
> the mpegts.h is missing the G_BEGIN_DECLS resulting in linkage error when
> trying to use it from cpp.

Fixed.
Comment 38 Edward Hervey 2014-02-06 15:15:04 UTC
Pushed.

commit dc968163da90ec093f80aa93d16dc95b864a344a
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Thu Feb 6 13:53:49 2014 +0100

    mpegts: Updated docs with new API

commit 6a5f1354b34a62adfdf78a1d8963adb2d59f7b9b
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Tue Nov 19 12:21:31 2013 +0100

    mpegtsmux: Use mpeg-ts lib for PAT/PMT sections
    
    Rewrite of the PAT/PMT section handling to use the mpeg-ts library

commit 93a8137be62075fbcfa2b21cc7dd13e25b8d70af
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Wed Nov 20 11:14:46 2013 +0100

    mpegtsmux: Add support for muxing SI tables
    
    The muxer is now able to include DVB sections in the transport stream.
    
    The si-interval property will determine how often the SI tables are
    muxed into the stream.
    
    The section is handled by the mpeg-ts library. Below is a small example
    that will include a Netork Information Table with a Network Name
    descriptor in the stream.
    
    GstMpegTsNIT *nit;
    GstMpegTsDescriptor *descriptor;
    GstMpegTsSection *section;
    GstElement *mpegtsmux;
    
    gst_mpegts_initialize ();
    
    nit = gst_mpegts_section_nit_new ();
    nit->actual_network = TRUE;
    
    descriptor = gst_mpegts_descriptor_from_dvb_network_name ("Network name");
    g_ptr_array_add (nit->descriptors, descriptor);
    
    section = gst_mpegts_section_from_nit (nit);
    
    // mpegtsmux should be retrieved from the pipeline
    gst_mpegts_section_send_event (section, mpegtsmux);
    gst_mpegts_section_unref (section);

commit b7d256b4c292d0a83f8c7eb2a02de9700082700b
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Tue Nov 19 11:30:33 2013 +0100

    mpegts: Support registration and custom descriptor
    
    Support for registration descriptor (0x05)
    Add function to create a descriptor with custom tag and data

commit 05bf952384b11fe2ab389b161c989522c3517b23
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Tue Nov 19 11:15:52 2013 +0100

    mpegts: Add creation of DVB Subtitling descriptor
    
    Descriptor tag is 0x59

commit cfb4da7215f96085ce167551382541ecdb99d4f7
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Tue Nov 19 10:50:30 2013 +0100

    mpegts: Support parsing of DVB Teletext descriptor
    
    Descriptor tag is 0x56

commit ffb51c2123fe62e0e62686d3d1bf643a5c034913
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Wed Nov 20 11:06:03 2013 +0100

    mpegts: Add support for creating PAT/PMT

commit fccfc76805beb7e467267eaed231308f7be98c31
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Wed Nov 20 11:04:58 2013 +0100

    mpegts: Add program_number to GstMpegTsPMT

commit dd449c38e6d0b88dd6c40e59a40c3a25ab8546e6
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Mon Oct 28 14:49:08 2013 +0100

    mpegts: Add network name descriptor construction
    
    Add function to create a Network Name DVB descriptor.

commit 8c26d69e2f6f75b2ed25239bed78cd86dfdfe4a5
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Mon Oct 28 14:48:13 2013 +0100

    mpegts: Add support for creating a NIT section
    
    Functions that will enable user to create Network Information Tables.

commit 4632ccd5bee5060fbca72d76df77d2abe182d47e
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Mon Oct 28 14:26:39 2013 +0100

    mpegts: Add network_id to GstMpegTsNIT
    
    The network_id is stored in the subtable extension. Make a field
    in the GstMpegTsNIT for better code readability

commit 8f429c6c6e0cc971566854e08f62fc11933a16ff
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Thu Oct 31 13:47:23 2013 +0100

    mpegts: Add functions to send sections as events
    
    Added function that enables the user to send a GstMpegTsSection as
    an event to a GstElement. (i.e. mpegtsmux)

commit 930cde73a7313d29335d93637e8609b612cf9df4
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Mon Oct 28 14:44:13 2013 +0100

    mpegts: Add functions to packetize section
    
    Sections needs to be packetized for use in i.e. mpegtsmux.
    These functions handles the generic common parts of a GstMpegTsSection

commit b1c5143b79be4285584a3905eeb8b7e0d42ddce9
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Mon Oct 28 14:39:49 2013 +0100

    mpegts: Add init functions for descriptor/section

commit 4630dfda0596f004609b5c753bf840d9ae10fbcd
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Wed Oct 30 10:43:21 2013 +0100

    mpegts: Add function to encode UTF8 strings
    
    This can be used to create descriptors with appropriate character maps

commit 465dea6f3252707886bce09f0d96500d3424b3b1
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Wed Oct 30 10:42:07 2013 +0100

    mpegts: Use big endian UCS-2 coding
    
    Parsing of control codes requires a big endian character map

commit 73c82e3f1c0e8f14365325a4e7d86907054dae4d
Author: Jesper Larsen <jesper.larsen@ixonos.com>
Date:   Wed Oct 30 10:33:18 2013 +0100

    mpegts: Add support for two-way iconv conversions
    
    To use in conversions from UTF-8 to another character map