[Bug 753614] qtdemux: PIFF box parsing support

[GStreamer (GNOME Bugzilla)]

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

Thiago Sousa Santos <thiagossantos at gmail.com> changed:

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

--- Comment #4 from Thiago Sousa Santos <thiagossantos at gmail.com> ---
Review of attachment 309251:
 --> (https://bugzilla.gnome.org/review?bug=753614&attachment=309251)

Some minor comments below.

Are there any public samples on this protection scheme?

::: gst/isomp4/qtdemux.c
@@ +2414,3 @@
+
+  structure = gst_caps_get_structure (stream->caps, 0);
+  g_assert (gst_structure_has_name (structure, "application/x-cenc"));

This doesn't look like a programming error. A malicious file could be crafted
to have a piff box on a regular non-ecnrypted file. I'd just use a warning here
and abort parsing the piff.

@@ +2511,3 @@
+    GstBuffer *box;
+
+    properties = qtdemux_get_cenc_sample_properties (qtdemux, stream, i);

Is sample here the number of samples on the stream or is this a different
meaning for sample?

Creating 1 structure per sample seems heavy. Perhaps use an array of custom
structs?

@@ +2539,3 @@
   };
+
+  static const guint8 piff_uuid[] = {

This not really a piff box, but a piff_sample_encryption_uuid. piff seems to be
the ftyp of the file.

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