Bug 180083 - [EME] Add support of multi keys from different sessions in CDMinstanceClearKey
Summary: [EME] Add support of multi keys from different sessions in CDMinstanceClearKey
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-28 08:38 PST by Yacine Bandou
Modified: 2018-01-23 04:08 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.89 KB, patch)
2017-11-28 08:55 PST, Yacine Bandou
no flags Details | Formatted Diff | Diff
Patch (3.71 KB, patch)
2018-01-17 06:01 PST, Yacine Bandou
no flags Details | Formatted Diff | Diff
Patch (4.26 KB, patch)
2018-01-18 05:23 PST, Yacine Bandou
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yacine Bandou 2017-11-28 08:38:39 PST
Add support of multi keys from different MediaKeySession in CDMinstanceClearKey

Use case:
Two sessions of MediaKeysession, one for the audio track and one for the video track and
a single CDMinstanceClearKey that receives the keys of both sessions,
so it should cache these keys in a vector in order to send them to the Decryptors
Comment 1 Yacine Bandou 2017-11-28 08:55:01 PST
Created attachment 327753 [details]
Patch
Comment 2 Xabier Rodríguez Calvar 2017-11-29 04:42:24 PST
Comment on attachment 327753 [details]
Patch

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

I think in this case we would be duplicating the keys so I'd suggest checking for the keys to be inside the Vector before adding a new one.

Another issue I see is that a little bit below, the vector is cleared and probably we don't want to clear the keys that are still associated with a session. I think this logic is flawed and needs more work.

Maybe Žan can correct me if I am wrong.

> Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:545
> +        for (auto& key : keyVector)
> +            m_keys.append(key);

If you wanted to do this, which I don't think is what I'd really want, you could use appendVector.
Comment 3 Yacine Bandou 2017-11-30 02:09:11 PST
(In reply to Xabier Rodríguez Calvar from comment #2)
> Comment on attachment 327753 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=327753&action=review
> 
> I think in this case we would be duplicating the keys so I'd suggest
> checking for the keys to be inside the Vector before adding a new one.
> 
I agree

> Another issue I see is that a little bit below, the vector is cleared and
> probably we don't want to clear the keys that are still associated with a
> session. I think this logic is flawed and needs more work.
> 

I'll check

> Maybe Žan can correct me if I am wrong.
> 
> > Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:545
> > +        for (auto& key : keyVector)
> > +            m_keys.append(key);
> 
> If you wanted to do this, which I don't think is what I'd really want, you
> could use appendVector.
Comment 4 Yacine Bandou 2018-01-17 06:01:51 PST
Created attachment 331486 [details]
Patch
Comment 5 Xabier Rodríguez Calvar 2018-01-17 23:47:53 PST
Comment on attachment 331486 [details]
Patch

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

Can you explain a bit more how this works?

> Source/WebCore/ChangeLog:8
> +        Add support of multi keys from different MediaKeySession in CDMinstanceClearKey.

CDMInstanceClearKey

> Source/WebCore/ChangeLog:9
> +        The single CDMinstanceClearKey that receives the keys of different sessions

ditto
Comment 6 Yacine Bandou 2018-01-18 05:23:08 PST
Created attachment 331614 [details]
Patch
Comment 7 WebKit Commit Bot 2018-01-23 04:07:47 PST
Comment on attachment 331614 [details]
Patch

Clearing flags on attachment: 331614

Committed r227409: <https://trac.webkit.org/changeset/227409>
Comment 8 WebKit Commit Bot 2018-01-23 04:07:48 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-01-23 04:08:30 PST
<rdar://problem/36771885>