[Bug 705991] Adding support for DASH common encryption to qtdemux and dashdemux

[GStreamer (GNOME Bugzilla)]

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

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

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

--- Comment #119 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 301696
  --> https://bugzilla.gnome.org/attachment.cgi?id=301696
event: Create a new Protection event

Also looks good, just a few minor niggles:

>+GstEvent *
>+gst_event_new_protection (const gchar * system_id,
>+    GstBuffer * data, const gchar * origin)
>+{
>+  ...
>+  event_name =
>+      g_strconcat ("GstProtectionEvent-",
>+      origin ? origin : "", "-", system_id, NULL);

If origin is NULL the event string contains "--", is that on purpose?

>+void
>+gst_event_parse_protection (GstEvent * event, const gchar ** system_id,
>+    GstBuffer ** data, const gchar ** origin)
>+{
>+  ...
>+  g_return_if_fail (GST_EVENT_TYPE (event) == GST_EVENT_PROTECTION);
>+
>+  s = gst_event_get_structure (event);
>+  name = gst_structure_get_name (s);
>+  g_return_if_fail (g_str_has_prefix (name, "GstProtectionEvent-"));

I don't think this name check is needed here, the EVENT_TYPE check should be
enough.

>+  if (origin)
>+    *origin = gst_structure_get_string (s, "origin");
>+
>+  if (system_id)
>+    *system_id = gst_structure_get_string (s, "system_id");
>+
>+  if (data) {
>+    const GValue *value = gst_structure_get_value (s, "data");
>+    g_return_if_fail (GST_VALUE_HOLDS_BUFFER (value));

Don't think this HOLDS_BUFFER check is needed here. We created the event after
all. (People can do crazy things of course, but we don't have to worry about
that)


>   GST_EVENT_SINK_MESSAGE          = GST_EVENT_MAKE_TYPE (100, FLAG(DOWNSTREAM) | FLAG(SERIALIZED) | FLAG(STICKY) | FLAG(STICKY_MULTI)),
>   GST_EVENT_EOS                   = GST_EVENT_MAKE_TYPE (110, FLAG(DOWNSTREAM) | FLAG(SERIALIZED) | FLAG(STICKY)),
>   GST_EVENT_TOC                   = GST_EVENT_MAKE_TYPE (120, FLAG(DOWNSTREAM) | FLAG(SERIALIZED) | FLAG(STICKY) | FLAG(STICKY_MULTI)),
>+  GST_EVENT_PROTECTION = GST_EVENT_MAKE_TYPE (130,
>+      FLAG (DOWNSTREAM) | FLAG (SERIALIZED) | FLAG (STICKY) |
>+      FLAG (STICKY_MULTI)),

Please align your addition with that of the other events.

And don't forget to add 'Since: 1.6' markers to the gtk-doc chunks.

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