Closed Bug 1415466 Opened 7 years ago Closed 7 years ago

[EME] Update content_decryption_module.h and content_decryption_module_ext.h for CDM version 1.4.9.xxxx

Categories

(Core :: Audio/Video: GMP, enhancement, P3)

Product:
Core
Component:
Audio/Video: GMP
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: JamesCheng, Assigned: JamesCheng)

References

Details

Attachments

(2 files)

Bug 1415466 - Part1 - Update content_decryption_module.h, there are no changes in the rest of headers.
59 bytes, text/x-review-board-request
cpearce
: review+
Details
Bug 1415466 - Part2 - Fix compile error due to the interface change.
59 bytes, text/x-review-board-request
cpearce
: review+
Details 
The content_decryption_module.h and content_decryption_module_ext.h header in chromium tip have a number of changes.

We should update our headers.

I will sync their headers when they release the CDM version 9 in their Canary to make sure the interface in the headers will not be modified for compatibility.
Change from

virtual void OnStorageId(const uint8_t* storage_id,
                           uint32_t storage_id_size) = 0;

to 

virtual void OnStorageId(uint32_t version,
                         const uint8_t* storage_id,
                         uint32_t storage_id_size) = 0;

the only API interface change currently.

Keep monitoring.
Hi Chris,

I have a question need your help and clarification.

I will call "content_decryption_module.h" as cdm_X_and_Y.h

Current we used cdm_8_and_9.h but not synced with upstream so I called it cdm_8_and_9_old.h (with CDM 8)

Chrome used cdm_8_and_9.h now (With CDM 8)

I assume it will become cdm_9_and_10.h when CDM version 9 released.

My questions are about the procedure to update the header.

We use "cdm_8_and_9_old.h" now and

1. we will request balrog rule to download CDM 9 one day. Therefore, we must sync our header from cdm_8_and_9_old.h to  cdm_8_and_9.h ```before``` balrog rule updated. Right?
  - That means I need to land this patch ```right now``` otherwise, chromium will update the header into 
    cdm_9_and_10.h.
2. Once we let Nightly download CDM 9 with cdm_8_and_9.h.
  - We should start to update from cdm_8_and_9.h to cdm_9_and_10.h, right?

I'm not sure if you met this situation before when you were dealing with bug 1381720.

But I'm afraid that I will mess up anything....

Please let me know if anything that I ignore considering.

Thanks
Flags: needinfo?(cpearce)
(In reply to James Cheng[:JamesCheng] from comment #3)
> Hi Chris,
> 
> I have a question need your help and clarification.
> 
> I will call "content_decryption_module.h" as cdm_X_and_Y.h
> 
> Current we used cdm_8_and_9.h but not synced with upstream so I called it
> cdm_8_and_9_old.h (with CDM 8)
> 
> Chrome used cdm_8_and_9.h now (With CDM 8)
> 
> I assume it will become cdm_9_and_10.h when CDM version 9 released.
> 
> My questions are about the procedure to update the header.
> 
> We use "cdm_8_and_9_old.h" now and
> 
> 1. we will request balrog rule to download CDM 9 one day. Therefore, we must
> sync our header from cdm_8_and_9_old.h to  cdm_8_and_9.h ```before``` balrog
> rule updated. Right?


Yes. We can only push out an interface 9 CDM to Firefox builds which have compatible headers for version 9.

So we need to either land the new header, or uplift patches with support for the new header.


>   - That means I need to land this patch ```right now``` otherwise, chromium
> will update the header into 
>     cdm_9_and_10.h.


I don't think that chromium will update the cdm.h header to include a version 10 quickly. The current header has been on version 8/9 for about a year IIRC.


> 2. Once we let Nightly download CDM 9 with cdm_8_and_9.h.
>   - We should start to update from cdm_8_and_9.h to cdm_9_and_10.h, right?

We may as well, as that makes it easier for us to ship newer CDMs.



> 
> I'm not sure if you met this situation before when you were dealing with bug
> 1381720.
> 
> But I'm afraid that I will mess up anything....

Be brave. We can backout any patch.
Flags: needinfo?(cpearce)
Blocks: 1417297
Attachment #8926294 - Flags: review?(cpearce)
Attachment #8928367 - Flags: review?(cpearce)
Thanks for your explanation.

So I'd like to update the header "before" the CDM 9 releases.

And I will confirm the header changes again before we want to push out CDM 9.
I just discovered the Chrome team mentioning a version 10 of CDM.h here:
https://bugs.chromium.org/p/chromium/issues/detail?id=570214
Comment on attachment 8926294 [details]
Bug 1415466 - Part1 - Update content_decryption_module.h, there are no changes in the rest of headers.

https://reviewboard.mozilla.org/r/197574/#review205188
Attachment #8926294 - Flags: review?(cpearce) → review+
Comment on attachment 8928367 [details]
Bug 1415466 - Part2 - Fix compile error due to the interface change.

https://reviewboard.mozilla.org/r/199576/#review205190
Attachment #8928367 - Flags: review?(cpearce) → review+
Pushed by jacheng@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6da5f175da3
Part1 - Update content_decryption_module.h, there are no changes in the rest of headers. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/348e3909c409
Part2 - Fix compile error due to the interface change. r=cpearce
https://hg.mozilla.org/mozilla-central/rev/a6da5f175da3
https://hg.mozilla.org/mozilla-central/rev/348e3909c409
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: