New issue
Advanced search Search tips

Issue 723357 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Add a flag to cover experimental CDM interface support

Project Member Reported by xhw...@chromium.org, May 17 2017

Issue description

Traditionally we add support of a new version of CDM interface in one CL, e.g. [1, 2]. Depending on the scale of the CDM interface change, the CL could be pretty big. 

Also, this requires that we stabilize the CDM interface before Chromium adds supports to it. After we start to ship Chrome versions that support a given CDM interface version, changing the CDM interface could cause incompatibility issues, e.g. a newer CDM used on an older Chrome version. Typically this isn't a big issue, especially since we never ship a real CDM before we finalize the CDM interface and Chromium's support of it, and we bundle the CDM on almost all platforms. But still there's a slight chance of incompatibility issue and if that happens it'll be hard to debug.

To solve these, I propose we add a new flag, e.g. --support-experimental-cdm-interface, and work on the support of a new CDM interface under this flag. Only when we have finalized the CDM interface and Chromium's support, that we enable the support of it by default. Before that, we can use this flag in tests so that we still have proper test coverage.

This approach has the following benefits:
- It allows us to develop the feature gradually, in small CLs, with test coverage.
- Even if we need to change the CDM interface, we can do so without any compatibility issues for normal users.

[1] https://codereview.chromium.org/1023003002
[2] https://codereview.chromium.org/811923002
 
Cc: -jrumm...@chromium.org xhw...@chromium.org
Labels: M-61
Owner: jrumm...@chromium.org
With the pepper plugin, it doesn't currently use base.dll or media.dll, so using regular feature flags won't work. Investigating if we can use compile time flags (so it can be turned on for ECK only).
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7edc791763ef56992687f32632011290bf967b0a

commit 7edc791763ef56992687f32632011290bf967b0a
Author: John Rummell <jrummell@chromium.org>
Date: Fri Jul 07 19:20:04 2017

Add support for CDM_9 interface

Adding support for CDM_9 interface. For mojo CDMs, this is only available
if the new flag "support-experimental-cdm-interface" is specified. For pepper
CDMs, a #define flag is used due to limited support for flags inside the
CDM adapter.

BUG= 723357 
TEST=locally with CDM_9 CDM

Change-Id: Ic54c7c51a2037ee1661ef5e52f00e24678fe855c
Reviewed-on: https://chromium-review.googlesource.com/554039
Commit-Queue: John Rummell <jrummell@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485004}
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/DEPS
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/base/media_switches.cc
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/base/media_switches.h
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/cdm_adapter.cc
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/cdm_adapter.h
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/cdm_wrapper.h
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/ppapi/BUILD.gn
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/ppapi/ppapi_cdm_adapter.cc
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/ppapi/ppapi_cdm_adapter.h
[modify] https://crrev.com/7edc791763ef56992687f32632011290bf967b0a/media/cdm/supported_cdm_versions.cc

Status: Fixed (was: Assigned)

Sign in to add a comment