Bug 162915 - [GStreamer][EME] Base class for decryption support
Summary: [GStreamer][EME] Base class for decryption support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrique Ocaña
URL:
Keywords:
Depends on:
Blocks: 157314
  Show dependency treegraph
 
Reported: 2016-10-04 10:19 PDT by Enrique Ocaña
Modified: 2016-10-26 01:48 PDT (History)
2 users (show)

See Also:


Attachments
Patch (21.43 KB, patch)
2016-10-04 10:25 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (21.41 KB, patch)
2016-10-16 12:11 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (21.42 KB, patch)
2016-10-26 01:23 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrique Ocaña 2016-10-04 10:19:47 PDT
Add common encryption base class.
Comment 1 Enrique Ocaña 2016-10-04 10:25:35 PDT
Created attachment 290611 [details]
Patch
Comment 2 Enrique Ocaña 2016-10-04 10:26:24 PDT
Comment on attachment 290611 [details]
Patch

Wait until all the patches in 157314 are ready.
Comment 3 WebKit Commit Bot 2016-10-04 10:28:09 PDT
Attachment 290611 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h:45:  webkit_media_common_encryption_decrypt_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:56:  webkit_media_common_encryption_decrypt_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:80:  webkit_media_common_encryption_decrypt_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Enrique Ocaña 2016-10-04 10:36:14 PDT
The style failures are on purpose, because of gobject macro machinery requirements.
Comment 5 Xabier Rodríguez Calvar 2016-10-08 08:59:41 PDT
Comment on attachment 290611 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290611&action=review

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:34
> +    GstEvent* protectionEvent;

GRefPtr

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:59
> +    GstBaseTransformClass* baseTransformClass = GST_BASE_TRANSFORM_CLASS(klass);
> +    GstElementClass* elementClass = GST_ELEMENT_CLASS(klass);

Move these to the latest before they are used.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:109
> +    GstCaps* transformedCaps = gst_caps_new_empty();
> +    WebKitMediaCommonEncryptionDecrypt* self = WEBKIT_MEDIA_CENC_DECRYPT(base);
> +    WebKitMediaCommonEncryptionDecryptClass* klass = WEBKIT_MEDIA_CENC_DECRYPT_GET_CLASS(self);

Move these after the GST_DEBUG_OBJECT

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:116
> +        GstStructure* in = gst_caps_get_structure(caps, i);
> +        GstStructure* out = nullptr;

incomingStructure, outgoingStructure, GRefPtr for outgoing

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:134
> +            GstStructure* tmp = gst_structure_copy(in);

GRefPtr

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:161
> +            out = gst_structure_copy(tmp);
> +            gst_structure_set(out, "protection-system", G_TYPE_STRING, klass->protectionSystemId,
> +                "original-media-type", G_TYPE_STRING, gst_structure_get_name(in), nullptr);
> +
> +            gst_structure_set_name(out, "application/x-cenc");
> +            gst_structure_free(tmp);

It does not make too much sense to me to copy the structure and free the original. Better to just assign it and avoid a copy.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:168
> +            GstStructure* s = gst_caps_get_structure(transformedCaps, index);

s -> structure

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:176
> +        if (!duplicate)
> +            gst_caps_append_structure(transformedCaps, out);
> +        else
> +            gst_structure_free(out);

If we want to use GRefPtr we probably want to use leakRef here in the if and remove the else.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:197
> +    guint subSampleCount, ivSize;

unsigned

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:202
> +    WebKitMediaCommonEncryptionDecrypt* self = WEBKIT_MEDIA_CENC_DECRYPT(base);
> +    WebKitMediaCommonEncryptionDecryptClass* klass = WEBKIT_MEDIA_CENC_DECRYPT_GET_CLASS(self);
> +    WebKitMediaCommonEncryptionDecryptPrivate* priv = WEBKIT_MEDIA_CENC_DECRYPT_GET_PRIVATE(self);
> +    guint subSampleCount, ivSize;
> +    gboolean encrypted;
> +    const GValue* value;
> +    GstBuffer* ivBuffer = nullptr;
> +    GstBuffer* subSamplesBuffer = nullptr;
> +    GstProtectionMeta* protectionMeta;

Could you please check this variables and move them to the latest place possible?

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:210
> +            GST_DEBUG_OBJECT(self, "can't process key requests in less than PAUSED state");

ERROR?

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:354
> +    GstStateChangeReturn ret = GST_STATE_CHANGE_SUCCESS;

ret -> result
Comment 6 Enrique Ocaña 2016-10-16 12:11:48 PDT
Created attachment 291770 [details]
Patch
Comment 7 WebKit Commit Bot 2016-10-16 12:14:45 PDT
Attachment 291770 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h:44:  webkit_media_common_encryption_decrypt_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:56:  webkit_media_common_encryption_decrypt_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:79:  webkit_media_common_encryption_decrypt_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Enrique Ocaña 2016-10-26 01:23:25 PDT
Created attachment 292900 [details]
Patch
Comment 9 WebKit Commit Bot 2016-10-26 01:29:40 PDT
Attachment 292900 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.h:44:  webkit_media_common_encryption_decrypt_get_type is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:56:  webkit_media_common_encryption_decrypt_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:79:  webkit_media_common_encryption_decrypt_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Enrique Ocaña 2016-10-26 01:48:47 PDT
Comment on attachment 292900 [details]
Patch

Clearing flags on attachment: 292900

Committed r207887: <http://trac.webkit.org/changeset/207887>
Comment 11 Enrique Ocaña 2016-10-26 01:48:55 PDT
All reviewed patches have been landed.  Closing bug.