[Bug 765275] matroska: implement reading & writing ContentEncryption headers

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #28 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 361953:
 --> (https://bugzilla.gnome.org/review?bug=765275&attachment=361953)

It still seems wrong to me to make the DRM system id optional in the protection
event, but what can we do. How would an application right now know what to
use/do?

::: gst/matroska/matroska-demux.c
@@ +380,3 @@
+
+    if (gst_matroska_parse_protection_meta (&data, &size, info_protect,
+            &encrypted)) {

How do we know that this is to be used, if it's only defined for WebM but not
general Matroska? Is there any indication in the file anywhere?

Also maybe the "Subsample Encrypted Block" should be passed directly downstream
as-is and the decryptor that handles "application/x-matroska-enc" is supposed
to handle that? Why would this be extracted at the demuxer level instead of
having it handled in the decryptor?

::: gst/matroska/matroska-read-common.c
@@ +361,3 @@
+gboolean
+gst_matroska_parse_protection_meta (gpointer * data_out, gsize * size_out,
+    GstStructure * info_protect, gboolean * encrypted)

Support for this should probably also be added to matroskamux, or at least it
should fail writing streams that require this according to the caps?

@@ +543,3 @@
+      gst_buffer_unref (buf_sub_sample);
+    } else {
+      gst_structure_set (info_protect, "subsample_count", G_TYPE_UINT, 0,
NULL);

Underscore instead of dash

@@ +3061,3 @@
+                    GST_DEBUG_OBJECT (common->sinkpad,
+                        "ContentEncAESSettings: %" G_GUINT64_FORMAT, num);
+                    enc.enc_CipherMode = num;

This new field does not seem to be used anywhere? It should probably also go
into the caps?

@@ +3100,3 @@
+
+              context->protection_info =
+                  gst_structure_new ("application/x-cenc", "iv_size",

We usually use dashes instead of underscores in structure field names

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