Add the support of WebM encrypted caps "application/x-webm-enc" Tests: media/encrypted-media/clearKey/clearKey-encrypted-webm-event-mse.html media/encrypted-media/clearKey/clearKey-webm-video-playback-mse.html
Created attachment 348768 [details] Patch
Comment on attachment 348768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348768&action=review Wrote some comments on https://bugzilla.gnome.org/show_bug.cgi?id=765275 cause I don't like the "UNDEFINED" API. > Source/WebCore/ChangeLog:11 > + see https://bugzilla.gnome.org/attachment.cgi?id=365211 Nit: There's a period missing at the end of this line.
Created attachment 351539 [details] Patch
Comment on attachment 351539 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351539&action=review > Source/WebCore/ChangeLog:11 > + in case of WebM, for details, see https://bugzilla.gnome.org/attachment.cgi?id=365211 period at the end. > Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:179 > + return (gst_structure_has_name(structure, "application/x-cenc") || gst_structure_has_name(structure, "application/x-webm-enc")); You don't need the () enclosing the two checks. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1295 > + if (eventKeySystemUUID == GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) > + weakThis->m_player->initializationDataEncountered("webm"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())); > + else > + weakThis->m_player->initializationDataEncountered("cenc"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())); You can use a ? b : c instead of an if. I have also a concern of what we are considering here: if system id is unspecified then we consider webm, cenc otherwise, which means that cenc needs to enforce a sys id and WebM will never have one. I guess the later is true by per spec but I wonder if that assumtion of cenc is true. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1296 > + This line should be removed. > Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:61 > + "application/x-cenc, original-media-type=(string)audio/mpeg, protection-system=(string)" WEBCORE_GSTREAMER_EME_UTILITIES_CLEARKEY_UUID";" > + "application/x-webm-enc, original-media-type=(string)video/x-vp8;" > + "application/x-webm-enc, original-media-type=(string)video/x-vp9;")); Can WebM handle cenc? If it can then we have an issue with this. > Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:165 > + if (!g_strcmp0(klass->protectionSystemId, GST_PROTECTION_UNSPECIFIED_SYSTEM_ID)) > + gst_structure_set_name(outgoingStructure.get(), "application/x-webm-enc"); > + else > + gst_structure_set_name(outgoingStructure.get(), "application/x-cenc"); Ditto for a ? b : c.
(In reply to Xabier Rodríguez Calvar from comment #4) > Comment on attachment 351539 [details] > Patch > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1295 > > + if (eventKeySystemUUID == GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) > > + weakThis->m_player->initializationDataEncountered("webm"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())); > > + else > > + weakThis->m_player->initializationDataEncountered("cenc"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())); > > You can use a ? b : c instead of an if. > > I have also a concern of what we are considering here: if system id is > unspecified then we consider webm, cenc otherwise, which means that cenc > needs to enforce a sys id and WebM will never have one. I guess the later is > true by per spec but I wonder if that assumtion of cenc is true. > CENC specifies the system ID and it is mandatory in PSSH Box. Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:61 > > + "application/x-cenc, original-media-type=(string)audio/mpeg, protection-system=(string)" WEBCORE_GSTREAMER_EME_UTILITIES_CLEARKEY_UUID";" > > + "application/x-webm-enc, original-media-type=(string)video/x-vp8;" > > + "application/x-webm-enc, original-media-type=(string)video/x-vp9;")); > > Can WebM handle cenc? If it can then we have an issue with this. > Why do you say that? It is "x-webm-enc" not "x-webm-cenc".
Created attachment 351599 [details] Patch
Comment on attachment 351599 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351599&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1292 > + (eventKeySystemUUID == GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) ? weakThis->m_player->initializationDataEncountered("webm"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())) : weakThis->m_player->initializationDataEncountered("cenc"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())); It would be much simpler to write: weakThis->m_player->initializationDataEncountered(eventKeySystemUUID == GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) ? "webm"_s : "cenc"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())); > Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:162 > + (!g_strcmp0(klass->protectionSystemId, GST_PROTECTION_UNSPECIFIED_SYSTEM_ID)) ? gst_structure_set_name(outgoingStructure.get(), "application/x-webm-enc") : gst_structure_set_name(outgoingStructure.get(), "application/x-cenc"); gst_structure_set_name(outgoingStructure.get(), g_strcmp0(klass->protectionSystemId, GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) ? "application/x-cenc" : "application/x-webm-enc");
(In reply to Yacine Bandou from comment #5) > Source/WebCore/platform/graphics/gstreamer/eme/ > WebKitClearKeyDecryptorGStreamer.cpp:61 > > > + "application/x-cenc, original-media-type=(string)audio/mpeg, protection-system=(string)" WEBCORE_GSTREAMER_EME_UTILITIES_CLEARKEY_UUID";" > > > + "application/x-webm-enc, original-media-type=(string)video/x-vp8;" > > > + "application/x-webm-enc, original-media-type=(string)video/x-vp9;")); > > > > Can WebM handle cenc? If it can then we have an issue with this. > > > > Why do you say that? It is "x-webm-enc" not "x-webm-cenc". What I mean is that if WebM supports cenc, we should provide a media type for application/x-cenc, original-media-type=(string)video/x-vp8 and vp9.
(In reply to Xabier Rodríguez Calvar from comment #8) > (In reply to Yacine Bandou from comment #5) > > Source/WebCore/platform/graphics/gstreamer/eme/ > > WebKitClearKeyDecryptorGStreamer.cpp:61 > > > > + "application/x-cenc, original-media-type=(string)audio/mpeg, protection-system=(string)" WEBCORE_GSTREAMER_EME_UTILITIES_CLEARKEY_UUID";" > > > > + "application/x-webm-enc, original-media-type=(string)video/x-vp8;" > > > > + "application/x-webm-enc, original-media-type=(string)video/x-vp9;")); > > > > > > Can WebM handle cenc? If it can then we have an issue with this. > > > > > > > Why do you say that? It is "x-webm-enc" not "x-webm-cenc". > > What I mean is that if WebM supports cenc, we should provide a media type > for application/x-cenc, original-media-type=(string)video/x-vp8 and vp9. No, it is not the same thing. For example: The PSSH box is mandatory in CENC protection scheme. If we set the caps "application/x-cenc, original-media-type=(string)video/x-vp9" for encrypted WebM/VP9. How we can differentiate it from encrypted MP4/VP9 ? For you information, MP4 supports the VP8 and VP9 codec (http://mp4ra.org/#/codecs). see also these two links: https://www.w3.org/TR/eme-stream-webm/ https://www.w3.org/TR/eme-stream-mp4/
(In reply to Xabier Rodríguez Calvar from comment #7) > Comment on attachment 351599 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351599&action=review > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1292 > > + (eventKeySystemUUID == GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) ? weakThis->m_player->initializationDataEncountered("webm"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())) : weakThis->m_player->initializationDataEncountered("cenc"_s, ArrayBuffer::create(reinterpret_cast<const uint8_t*>(initData.characters8()), initData.sizeInBytes())); > > It would be much simpler to write: > > weakThis->m_player->initializationDataEncountered(eventKeySystemUUID == > GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) ? "webm"_s : "cenc"_s, > ArrayBuffer::create(reinterpret_cast<const > uint8_t*>(initData.characters8()), initData.sizeInBytes())); > Absolutely.
Created attachment 351765 [details] Patch
(In reply to Yacine Bandou from comment #9) > > What I mean is that if WebM supports cenc, we should provide a media type > > for application/x-cenc, original-media-type=(string)video/x-vp8 and vp9. > > No, it is not the same thing. > > For example: The PSSH box is mandatory in CENC protection scheme. > > If we set the caps "application/x-cenc, > original-media-type=(string)video/x-vp9" for encrypted WebM/VP9. How we can > differentiate it from encrypted MP4/VP9 ? > For you information, MP4 supports the VP8 and VP9 codec > (http://mp4ra.org/#/codecs). This is exactly my question, how can we differentiate them. I guess you're assuming that in the case of mp4 with VP8/9 what the demuxer would report would be application/x-cenc, original-media-type=(string)video/mpeg ?
Comment on attachment 351765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351765&action=review > Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:163 > + (!g_strcmp0(klass->protectionSystemId, GST_PROTECTION_UNSPECIFIED_SYSTEM_ID)) ? "application/x-webm-enc" : "application/x-cenc"); You don't need the () around g_strcmp0.
(In reply to Xabier Rodríguez Calvar from comment #12) > (In reply to Yacine Bandou from comment #9) > > > What I mean is that if WebM supports cenc, we should provide a media type > > > for application/x-cenc, original-media-type=(string)video/x-vp8 and vp9. > > > > No, it is not the same thing. > > > > For example: The PSSH box is mandatory in CENC protection scheme. > > > > If we set the caps "application/x-cenc, > > original-media-type=(string)video/x-vp9" for encrypted WebM/VP9. How we can > > differentiate it from encrypted MP4/VP9 ? > > For you information, MP4 supports the VP8 and VP9 codec > > (http://mp4ra.org/#/codecs). > > This is exactly my question, how can we differentiate them. I guess you're > assuming that in the case of mp4 with VP8/9 what the demuxer would report > would be application/x-cenc, original-media-type=(string)video/mpeg ? Encrypted MP4/VP9 --> qtdemux with src caps "application/x-cenc, original-media-type=(string)video/VP9". Encrypted WebM/VP9 --> Matroskademux with src caps "application/x-webm-enc, original-media-type=(string)video/VP9".
Created attachment 351771 [details] Patch
Comment on attachment 351771 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351771&action=review > Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:61 > GST_STATIC_CAPS("application/x-cenc, original-media-type=(string)video/x-h264, protection-system=(string)" WEBCORE_GSTREAMER_EME_UTILITIES_CLEARKEY_UUID "; " > - "application/x-cenc, original-media-type=(string)audio/mpeg, protection-system=(string)" WEBCORE_GSTREAMER_EME_UTILITIES_CLEARKEY_UUID)); > + "application/x-cenc, original-media-type=(string)audio/mpeg, protection-system=(string)" WEBCORE_GSTREAMER_EME_UTILITIES_CLEARKEY_UUID";" > + "application/x-webm-enc, original-media-type=(string)video/x-vp8;" > + "application/x-webm-enc, original-media-type=(string)video/x-vp9;")); (In reply to Yacine Bandou from comment #14) > Encrypted MP4/VP9 --> qtdemux with src caps "application/x-cenc, > original-media-type=(string)video/VP9". Again, why did you decide to leave outside the question "application/x-cenc, original-media-type=(string)video/VP9" cause from current code I see it is not supported. Well, anyway, this would not belong to this patch, so I'll r+ it.
Comment on attachment 351771 [details] Patch Clearing flags on attachment: 351771 Committed r236912: <https://trac.webkit.org/changeset/236912>
All reviewed patches have been landed. Closing bug.