[Bug 761700] basedrm: a base class based on CENC common encryption scheme for DRM playback

[GStreamer (GNOME Bugzilla)]

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

Olivier Crête <olivier.crete at ocrete.ca> changed:

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

--- Comment #17 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 330703:
 --> (https://bugzilla.gnome.org/review?bug=761700&attachment=330703)

Having a library that only appears if you have some library installed, maybe we
could use GstURIDownloader instead? Also, your code has a undeclared dep on
libxml2.

I see HTTP SOAP requests and parsing of an XML file, what spec are those from?
Can you add that to the doc at the top? I know PlayReady uses SOAP, but do
Widevine, Marlin, etc?

Also you put stubs for all the functions, you may want to not put a stub for
the functions that really need to be implemented, and instead if g_assert() on
their presence.

::: gst-libs/gst/basedrm/gstbasedrm.c
@@ +293,3 @@
+  drm_license_info->response_length = 0;
+
+  proxy = g_getenv ("http_proxy");

This is not standard. LibSoup should just get the proxy config for you.
Otherwise, if you insist on a "manual", add a "proxy" property like
souphttpsrc.

@@ +458,3 @@
+      gst_structure_remove_field (fields, field_name);
+    }
+  }

Same here, you can just do gst_structure_remove_fields(fields, "base-profile",
"codec_data", ..., NULL);

@@ +511,3 @@
+          gst_structure_get_string (out, "original-media-type"));
+
+      /* filter out the DRM related fields from the down-stream caps */

Comment is upside down.. the DRM fields are in the upstream caps (upstraem is
towards the source)..

@@ +520,3 @@
+            g_str_has_prefix (field_name, "original-media-type")) {
+          gst_structure_remove_field (out, field_name);
+        }

The whole for() loop can be replace with gst_structure_remove_fields(in,
"protection-system", "original-media-type", NULL);

@@ +1057,3 @@
+        GST_DEBUG_OBJECT (basedrm, "event carries pssh data from qtdemux");
+        ret = gst_basedrm_parse_pssh_box (basedrm, pssi);
+      } else if (g_str_has_prefix (loc, "smooth-streaming")) {

In many places where you use g_str_has_prefix() you probably just can !strcmp()
instead.

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