WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
162915
[GStreamer][EME] Base class for decryption support
https://bugs.webkit.org/show_bug.cgi?id=162915
Summary
[GStreamer][EME] Base class for decryption support
Enrique Ocaña
Reported
2016-10-04 10:19:47 PDT
Add common encryption base class.
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
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Enrique Ocaña
Comment 1
2016-10-04 10:25:35 PDT
Created
attachment 290611
[details]
Patch
Enrique Ocaña
Comment 2
2016-10-04 10:26:24 PDT
Comment on
attachment 290611
[details]
Patch Wait until all the patches in 157314 are ready.
WebKit Commit Bot
Comment 3
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.
Enrique Ocaña
Comment 4
2016-10-04 10:36:14 PDT
The style failures are on purpose, because of gobject macro machinery requirements.
Xabier Rodríguez Calvar
Comment 5
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
Enrique Ocaña
Comment 6
2016-10-16 12:11:48 PDT
Created
attachment 291770
[details]
Patch
WebKit Commit Bot
Comment 7
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.
Enrique Ocaña
Comment 8
2016-10-26 01:23:25 PDT
Created
attachment 292900
[details]
Patch
WebKit Commit Bot
Comment 9
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.
Enrique Ocaña
Comment 10
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
>
Enrique Ocaña
Comment 11
2016-10-26 01:48:55 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug