[Bug 757049] tsdemux: Add support for Opus

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=757049

Tim-Philipp Müller <t.i.m at zen.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #314011|none                        |reviewed
             status|                            |

--- Comment #3 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 314011
  --> https://bugzilla.gnome.org/attachment.cgi?id=314011
tsdemux: Add support for Opus

(Did not check against the draft spec, just commenting on the code)

>+  packet_size = au_size - gst_byte_reader_get_pos (&reader);
>+  if (!gst_byte_reader_dup_data (&reader, packet_size, &packet_data))
>+    goto error;

I think an explicit check here that au_size is sane would be nicer, instead of
guint underflow -> huge value -> dup_data() fails.


>+  caps = gst_pad_get_current_caps (stream->pad);
>+  s = gst_caps_get_structure (caps, 0);
>+  if (!gst_structure_get (s, "streamheader", GST_TYPE_BUFFER, &streamheader,
>+          NULL)) {
>+    gst_caps_unref (caps);
>+    goto error;
>+  }
>+
>+  /* FIXME: Do something with end trim */
>+  gst_buffer_map (streamheader, &map, GST_MAP_READ);
>+  {
>+    guint16 pre_skip = GST_READ_UINT16_BE (map.data + 10);
>+
>+    if (pre_skip != start_trim) {
>+      new_streamheader = gst_buffer_copy (streamheader);
>+      start_trim = GUINT16_TO_BE (start_trim);
>+      gst_buffer_fill (new_streamheader, 10, &start_trim, 2);
>+    }
>+  }
>+
>+  gst_buffer_unmap (streamheader, &map);
>+  gst_buffer_unref (streamheader);
>+  if (new_streamheader) {
>+    caps = gst_caps_make_writable (caps);
>+    gst_caps_set_simple (caps,
>+        "streamheader", GST_TYPE_BUFFER, new_streamheader, NULL);
>+    gst_pad_set_caps (stream->pad, caps);
>+    gst_buffer_unref (new_streamheader);
>+  }
>+
>+  gst_caps_unref (caps);
>+
>+  return;
>+...
>+  }
>+}
>+
> static GstFlowReturn
> gst_ts_demux_push_pending_data (GstTSDemux * demux, TSDemuxStream * stream)
> {
>   ...
> 
>+  if (bs->stream_type == GST_MPEGTS_STREAM_TYPE_PRIVATE_PES_PACKETS &&
>+      bs->registration_id == DRF_ID_OPUS) {
>+    parse_opus_access_unit (stream);
>+  }
>+

This all looks a bit confusing to me. Are we doing caps/streamheader stuff for
every single packet? Potentially changing the streamheader for every single
packet?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.