Add the "cenc" initDataType support in ClearKey CDM. Parse the CENC initData and extract the KIDs by following the W3C spec https://www.w3.org/TR/eme-initdata-cenc/#common-system
Created attachment 327751 [details] Patch
Comment on attachment 327751 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327751&action=review > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:50 > +const unsigned ClearKeyCencSystemIdSize = sizeof(ClearKeyCencSystemId) / sizeof(uint8_t); Is there any architecture where sizeof(uint8_t) is not 1? If you know of any, leave it, if not, please remove. > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:168 > + Ref<SharedBuffer> keyids = SharedBuffer::create(); keyIds > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:207 > + unsigned nbKeyId = data[index + 3]; The variable name should full words. > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:208 > + index += 4; // KID_count size. Why KID_count with an underscore? > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:210 > + // check the overflow. Check > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:215 > + auto kidsArray = InspectorArray::create(); keyIdsArray > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:217 > + // Read the KeyID Missing period. > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:372 > + if (!(isKeyids || isCenc)) if (!isKeyids && !isCenc) > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:380 > + if (isCenc && (initData.isEmpty())) I wonder if we should actually parse the initData instead of just ensuring it is not empty. I think we should and for that we need to split the extraction function into two because just for consistency you don't need to create the JSON, just just need the information of how many keys are there and where they are located. So I'd write the following functions: * std::pair<unsigned, unsigned> extractKeyidsLocationFromCencInitData(...) that returns the number of keys and the position in the SharedBuffer data. * isCencInitData that invokes the first and checks keys are not zero. * extractKeyidsFromCencInitData that uses the first and continues the algorithm by extracting the keys and creating the JSON. > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:383 > return true; I think we should change the logic here, we consider it's a failure by default unless we manage to ensure initDatas check out.
And again, I forgot to ask the same question. Are there any tests for this?
(In reply to Xabier Rodríguez Calvar from comment #2) > Is there any architecture where sizeof(uint8_t) is not 1? If you know of > any, leave it, if not, please remove. Surely not any that can run glib or WebKit.
(In reply to Xabier Rodríguez Calvar from comment #3) > And again, I forgot to ask the same question. Are there any tests for this? I am working to add a ClearKey tests. I rework the patch as soon as possible.
Created attachment 329713 [details] Patch
Comment on attachment 329713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329713&action=review > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:50 > +const uint8_t ClearKeyCencSystemId[] = { 0x10, 0x77, 0xef, 0xec, 0xc0, 0xb2, 0x4d, 0x02, 0xac, 0xe3, 0x3c, 0x1e, 0x52, 0xe2, 0xfb, 0x4b }; > +const unsigned ClearKeyCencSystemIdSize = sizeof(ClearKeyCencSystemId); > +const unsigned KIDSize = 16; These things are variables, their names should begin with non-capital. I guess the naming rules would require that KIDSize -> keyIdSize; > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:172 > + if (initData.isEmpty() || initData.size() > std::numeric_limits<unsigned>::max()) There are two spaces before initData.size(). > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:185 > + if (index + 12 + ClearKeyCencSystemIdSize >= initDataSize) There are two spaces before ClearKeyCenc... > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:208 > + keyIdsMap.first = data[index + 3]; // Read the KeyIdsCount Missing period at the end of the comment. > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:220 > +// This function checks if the initData sharedBuffer is a validate CENC initData. ...is a valid... > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:251 > + String keyID = WTF::base64URLEncode(&data[index], KIDSize); keyId
Created attachment 330125 [details] Patch
Comment on attachment 330125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330125&action=review > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:172 > + if (initData.isEmpty() || initData.size() > std::numeric_limits<unsigned>::max()) (In reply to Xabier Rodríguez Calvar from comment #7) > > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:172 > > + if (initData.isEmpty() || initData.size() > std::numeric_limits<unsigned>::max()) > > There are two spaces before initData.size(). Not addressed.
Created attachment 330715 [details] Patch
Comment on attachment 330715 [details] Patch Clearing flags on attachment: 330715 Committed r226621: <https://trac.webkit.org/changeset/226621>
All reviewed patches have been landed. Closing bug.
<rdar://problem/36372226>