New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 597443 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: 2016-12-02
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

Can't transition between encrypted and unencrypted audio

Project Member Reported by joeyparrish@chromium.org, Mar 23 2016

Issue description

Chrome Version       : 50.0.2661.32 (Offizieller Build) beta (64-Bit)
URLs (if applicable) : http://horcrux.kir/audio-encryption-changes/demo/
OS version               : ChromeOS beta
Audio/Video format (if applicable): DASH/MSE/EME

What steps will reproduce the problem?
(1) Visit demo page at URL above
(2) Click "Load"
(3) Wait for video to start
(4) Seek to any time in the second period (13+ minutes, for example)
(5) On append of init data, playback fails and media-internals says "audio encryption changes not allowed"

What is the expected result?
The encryption scheme did not change, only the key IDs did.  So I expect the second period's init segment to be accepted and playback to continue into the second period.

What is the actual result?
Playback stops due to this code:
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/source_buffer_stream.cc&q=%22audio%20encryption%20changes%22&sq=package:chromium&l=1481
introduced by this change:
https://chromium.googlesource.com/chromium/src/+/8d5275f7bbb6f79c83829a7931f1500128ef3f8f%5E%21/#F43
 
Components: -Internals>Media Internals>Media>Encrypted

Comment 2 by xhw...@chromium.org, Mar 24 2016

Labels: ReleaseBlock-Stable M-50 OS-All
Owner: xhw...@chromium.org
Status: Assigned (was: Unconfirmed)
Thanks for reporting this. I'll take a look tomorrow.

Comment 3 by xhw...@chromium.org, Mar 24 2016

The suspected change [1] is only in M51. If this repros on M50 then it must be a different reason.

Is it possible that you are mixing unencrypted stream with encrypted ones?

[1] https://chromium.googlesource.com/chromium/src/+/8d5275f7bbb6f79c83829a7931f1500128ef3f8f%5E%21/#F43
I think I was hasty to file this.  I see the same failure on Chrome 49.  Let me dig into the init segments and find out.
Labels: -Pri-1 -Type-Bug Pri-3 Type-Feature
Summary: Can't transition between encrypted and unencrypted audio (was: Multi-period encrypted DASH fails: "audio encryption changes not allowed")
You are right.  The en-US track for period 2 is unencrypted.  It would seem that the content provider did this intentionally as a compatibility test:  https://github.com/Axinom/dash-test-vectors#v6-multidrm-multikey-multiperiod

I'm very sorry for wasting everyone's time by calling this a P1 bug.

Out of curiosity, is there any reason we can't switch between encrypted and unencrypted content?  From an application's point of view, this is not any different than encrypted content with a clear lead, which we definitely support.
I don't know the technical details of this specific code, but we do not support going from unencrypted to encrypted because we may need to use a different decoder. In addition to potential issues with inter frames (not sure if MSE would address this), it could also require us to rebuild the media pipeline.

Comment 7 by xhw...@chromium.org, Mar 24 2016

What ddorwin@ said. Support switching decoders involves a lot of work. Since this use case is a bit trivial we decided not to support it.
Why does this require switching decoders?  I've already attached MediaKeys, so shouldn't the CDM be decoding both periods regardless?  Isn't that the case when we have a clear lead?

Comment 9 by xhw...@chromium.org, Mar 24 2016

The JS isn't required to attach a MediaKeys immediately. Imagine you append unencrypted content and start playback without calling setMediaKeys(). The media pipeline would have no idea it could be encrypted later so it'll just use a normal decoder. Then you start to call setMediaKeys() and start to append encrypted content. At this point, we have to switch to a different decoder to be able to continue the playback.
Cc: -xhw...@chromium.org
Labels: -M-50 -ReleaseBlock-Stable
Status: Available (was: Assigned)
Removing milestone and stable blocker.

joeyparrish: Do you see any strong reason this would be a useful use case?
Perhaps I have done a bad job of communicating the situation.  What you're describing in comment 9 isn't what's happening here.  In my case, I see no reason to switch decoders.

We start with MediaKeys attached before appending anything, and we start by appending encrypted content.  Later, we try to append clear content.  I would expect that this would not require switching decoders.

I suspect, but have not confirmed, that if the resolutions and profiles matched between the encrypted & unencrypted content, I could switch to clear segments but omit the clear stream's init segment, and Chrome would continue decoding clear content through the CDM.  This would obviously be a hack, but I think if successful, would demonstrate that Chrome has the capability.

Thoughts?
Oh, sorry I was trying to think about a more general case.

In your specific case, we could relax the logic in the demuxer so that appending clear content after appending encrypted content is allowed, which should work in our current media stack. But this code has been here for so long that I suspect we have some other places with similar assumption that need to be fixed.

So from a real-world-impact's perspective, what's the use case for this? We need to evaluate it to decide whether we want to support this.
While this could be made to work if you know what you're doing, it might be unexpected that this works but the opposite order does not. If this was part of an adaptive implementation, playback might appear to "randomly" fail or only fail for certain users.

Perhaps a better option would be to allow this iff the MediaKeys was attached before the pipeline and deciders were initialized.
FWIW, currently demuxer knows nothing about MediaKeys/CDM. Plumbing this through would not be trivial.


Okay, I think I understand.  Thanks for considering it, but I'll go ahead and close.
Status: WontFix (was: Available)

Comment 17 by s...@philo.com, Jun 9 2016

I think there is a very real use case for this, which is out-of-band dynamic ad insertion. You may be serving DRMed content from your own packager, and then grab an unencrypted ad from an external ad server.
I opened an issue on Shaka for this here: https://github.com/google/shaka-player/issues/409
If this is a capability you want to use, you will probably want it to be consistently implemented and/or detectable across browsers. I suggest filing a feature request for EME at https://github.com/w3c/encrypted-media/issues/new. That will allow a broader discussion so that we can agree on and document something that works for most/all clients.

Note that there may be other issues, such as codec or container switching, which may also be problematic or require early knowledge of the codecs that will be used.

Comment 19 by s...@philo.com, Jun 11 2016

I've entered a feature request here: https://github.com/w3c/encrypted-media/issues/251

Out of curiosity, why do you think that this should live in the EME and not the demuxer itself? This may be my naive understanding, but shouldn't the demuxer honor the senc box if it exists, and treat the sample accordingly? Is it just a matter of making sure this is implemented consistently across browsers?

I totally understand why switching containers/codecs would be much more challenging, but it seems like a lower priority: I think it is easier to ask that an ad server comply with a given profile than ask that ads be encrypted in the same manner as the parent content.

We also have same problem for Ad-insertion, when switching between DASH periods from unencrypted ad content periods to encrypted main content periods and vice versus.

Since we do transcode ad and main contents with the same profile, so there were no problem switching between them on the fly when there are unencrypted.

Can I ask if it's possible to use ClearKey for Ads and main contents with PlayReady/Winewide? or is it the same issue with switching between clear and encrypted streams?

Currently we need to encrypt Ads the same way as main contents, but it add much more cost to request keys for ADs

Sorry for the typo, Widevine instead
You can only change key systems when you change video src attribute, so you can't change from Widevine to ClearKey and back if you want seamless playback across Periods.
Joey is correct. Note that the frames don't actually need to be encrypted - the container just needs to indicate that it may contain encrypted content. Thus, as a workaround, you could package the ads with Initialization Data and everything else that indicates there may be encrypted frames but not actually encrypt any blocks. Then you don't need to manage or request any additional keys. (This is not an ideal solution, but it should work across current browsers.)
Thank you so much for the invaluable input, ddorwin! I'll try it now. Did you tried that yourself?

No, I have not tried it, but it should be no different than a really long clear lead. You'll still get "encrypted" events for the ads, but you can ignore them.

Comment 26 by hangu...@gmail.com, Jul 14 2016

Hi again, I've been trying different packaging of the segments last few days, and so far the attached audio (a64) and video (v500) segments seem to be able to play by shaka without a key/keyRequest. The player can also switch back and forth to another full encrypted stream period. Sometimes it fails to switch between "clear" and encrypted periods, just frozen at last picture of current period. But I could seek backward or forwards to continue the switch between "clear" and encrypted ones. No errors from media-internals, pipeline_state is still in playing state.

Could you see if the attached initialization segments and the segments match with what you expected, ddorwin?

Thanks, /ha
clear_cenc.tgz
1.7 MB Download
I don't have any expectations - it was just a high level suggestion. Have you tried this in other browsers?

If you are having troubles with a specific set of streams, I suggest opening a new issue with repro steps or a link along with results in other browsers.
Owner: chcunningham@chromium.org
Status: Assigned (was: WontFix)
I'd like to take another look at making this work in Chrome. Joey/Seth, can you link me to a dash manifest or webpage that demonstrates the issue?
Cc: mlamouri@chromium.org
Axinom has a public test vector that demonstrates this issue: https://github.com/Axinom/dash-test-vectors

The MultiDRM-MultiKey-MultiPeriod stream has three audio languages with different transition types.  To make it easier, I have uploaded a Shaka Player demo with this asset selected by default.

For 'en', the first period is encrypted and the second is clear.
https://transitionbug-dot-shaka-player-demo.appspot.com/demo/?lang=en;play

For 'en-AU', the first period is clear and the second is encrypted.
https://transitionbug-dot-shaka-player-demo.appspot.com/demo/?lang=en-AU;play

For 'en-ET', both periods are encrypted.
https://transitionbug-dot-shaka-player-demo.appspot.com/demo/?lang=en-ET;play

To test the transition, seek to 11:40 (video.currentTime = 700).

In all three cases above, the video is encrypted, so Shaka Player has attached MediaKeys to the video element from the very beginning.  Nothing about the MediaKeys setup changes at transition time.

I hope that is helpful.
Thanks, very helpful. 
All of these transitions are working in both Edge and Firefox on Windows.
Labels: -Pri-3 Hotlist-Interop Pri-2
Thanks for confirming. Bumping the priority given the interop issue.

Comment 34 by faniel@google.com, Oct 6 2016

Cc: xhw...@chromium.org
Labels: EME-compat
This seems like a very narrow use case - video is always encrypted but audio switches.

Comment 17 seems more useful, but has several variants (setMediaKeys completes before loading the source, encrypted before clear, clear before encrypted). I'm curious about the behavior of browsers in those cases.

It would be nice to have spec tests for this. Will someone file a test request at https://github.com/w3c/web-platform-tests/issues/new and/or contribute a test that covers the use case(s)?

Let me try to rephrase this into a relatively simple request.

If MediaKeys was attached before any content was appended, then I expect to be able to append init segments for clear MP4 content and encrypted MP4 content in any order.  This is critical to ad insertion so that ads need not be encrypted.

Since I created a CDM instance early, and that CDM is already capable of decoding both clear and encrypted content (such as encrypted content with a clear lead), there's no reason to change decoders.

I do not expect to be able to change between CTR and CBC.


This works fine on MS Edge w/ PlayReady and Firefox w/ Widevine.

Here's the demo again.  Playback starts in an encrypted period.  Seek to the second half for an unencrypted period. https://transitionbug-dot-shaka-player-demo.appspot.com/demo/?lang=en;play


I think this code in src/media/filters/source_buffer_stream.cc:

  if (!audio_configs_[0].encryption_scheme().Matches(
          config.encryption_scheme())) {
    MEDIA_LOG(ERROR, media_log_) << "Audio encryption changes not allowed.";
    return false;
  }

Should change into something like this:

  if (config.encryption_scheme().is_encrypted()) {
    if (!config.encryption_scheme().Matches(audio_encryption_scheme_)) {
      MEDIA_LOG(ERROR, media_log_) << "Audio encryption changes not allowed.";
      return false;
    }
    // Remember this encryption scheme.  This is either the first encrypted
    // config we've seen, or it's identical to the first one we saw.
    audio_encryption_scheme_ = config.encryption_scheme();
  }

Same for video.  This would allow changes to and from unencrypted in any order, but would not allow changes from CBC to CTR, even with unencrypted in between.

Thoughts?
There was a bug in the code I proposed.  I think this is better:

  if (config.encryption_scheme().is_encrypted()) {
    if (audio_encryption_scheme_.is_encrypted() &&
        !config.encryption_scheme().Matches(audio_encryption_scheme_)) {
      MEDIA_LOG(ERROR, media_log_) << "Audio encryption changes not allowed.";
      return false;
    }
    // Remember this encryption scheme.  This is either the first encrypted
    // config we've seen, or it's identical to the first one we saw.
    audio_encryption_scheme_ = config.encryption_scheme();
  }
I have a local change in the chromecast tree (haven't started to upstream it yet) but it works for us:

 if (!audio_configs_[0].encryption_scheme().Matches(config.encryption_scheme())
#if defined(CHROMECAST_BUILD)
      && config.encryption_scheme().is_encrypted()
#endif

Our experience is that with a clear leader, the initial config still has is_encrypted true, the clear buffers having an empty key_id. So the only thing we needed to accommodate for was a new unencrypted config after an initial encrypted config.

But your change would work too. Happy to support any change that allows our unencrypted ad insertion scenario.
Re #36-#37: The request makes sense. I think we should support it. Since this seems to be a common pattern now.

But I don't see how your proposed changes would work. If the first init segment is clear, the current DecoderSelector [1] will try to select a clear decoder which doesn't use the CDM. And today we don't support switching decoders at run time so we'll not be able to support encrypted streams.

I think supporting switching decoders is doable. We can look at this in Q1. Or as for the specific example you give, we can change the DecoderSelector logic to always choose DecryptingVideoDecoder if there is a CDM attached.

For ChromeCast this is not an issue because it doesn't use DecoderSelector.

How urgent is this? Is M58 acceptable?

[1] https://cs.chromium.org/chromium/src/media/filters/decoder_selector.cc?rcl=0&l=110
I was assuming that attaching MediaKeys before appending the first init segment would cause the CDM's decoder to be selected.  How about a two-stage plan:

1. Assuming that the decrypting decoder has been selected already, support switching between that scheme and unencrypted.  This is the basic intent of my snippet, and the most important thing we can change.  Without it, nobody can deliver unencrypted ads in Chrome seamlessly.

2. If MediaKeys is attached, select the decrypting decoder, regardless of whether the first init segment was encrypted.  This could be done independent of the first part.  This, in addition to supporting switching between encrypted and unencrypted, would allow for clear pre-roll ads followed by encrypted content.

I don't see any urgent need to support changing decoders at all.  The difficulty in changing decoders was brought up originally as a justification for not supporting this feature, but I don't think it's relevant.

People have been advocating for this since June.  Neither Firefox nor Edge have this restriction.  The fix is small, and already exists on Chromecast.  Why wait until M58 to fix it?
Cc: -xhw...@chromium.org chcunningham@chromium.org
Labels: M-57
Owner: xhw...@chromium.org
Status: Started (was: Assigned)
Attaching MediaKeys will not trigger any decoder to be selected.

If we only do (1) today on desktop Chrome, we'll hit a decode error when you switch from clear ads to encrypted video. So I do feel we need to do (1) and (2) together.

One thing I am not super comfortable is that the behavior becomes different between attaching MediaKeys before and after play(). How are we gonna tell web developers that they have to attach MediaKeys before play() to get ads working?

That's the main reason I still like the idea of switching decoders, which would be a much cleaner solution. Also we have other projects that can benefit from being able to switch decoders.

Again, ChromeCast use a different Renderer so they only doing (1) works for them.

Actually, by looking at the currently implementation, supporting switching from a clear decoder to a decrypting decoder might not be hard at all. I'll play with it and let you know how that goes.

Let me mark this as M57 for now. But I can't promise it since I am fully occupied.
joeyparrish: Do you also have test page to switch from clear to encrypted, with MediaKeys set before AND after play()?
I'm all for doing a complete fix, but can we do (1) as a short-term solution?  To me, that is strictly better than what we have today, and it's trivial.
I don't have a test page that sets MediaKeys after play() because Shaka Player doesn't do that.  We always attach MediaKeys before any content is appended to the SourceBuffer.
Sure. If I can't do a full fix by end of next week, I'll land (1) first.

Do you have a test page to switch from clear to encrypted?
This transitions from encrypted to clear at the half-way point:
https://transitionbug-dot-shaka-player-demo.appspot.com/demo/?lang=en;play

This transitions from clear to encrypted at the half-way point:
https://transitionbug-dot-shaka-player-demo.appspot.com/demo/?lang=en-AU;play


Thank you!
Also, this test content comes from Axinom.  If you need any detailed info about these streams, it's here:

https://github.com/Axinom/dash-test-vectors/tree/e689891f#v6-multidrm-multikey-multiperiod
NextAction: 2016-12-02
Cc: erickung@chromium.org x...@chromium.org m...@chromium.org
erickung, miu, xjz: Do you see any issue for remoting if we allow switching between clear and encrypted streams during playback?

Comment 52 by m...@chromium.org, Nov 24 2016

I expect, if the issue is fixed in the normal Chrome media stack, then Remoting in the sender and receiver (the Chromecast) should pick up those changes as well. Otherwise, we did make a few assumptions about "never being able to switch between" in our control logic. So, we'll need to update our logic to account for this.

FWIW, we're initially launching w/o encrypted content support (M57 I hope); so don't let us block you. In other words, it's highly unlikely we would have a problem w/ you breaking any existing EME-related remoting functionality.

Comment 53 by x...@chromium.org, Nov 24 2016

Our remoting control logic needs to get notified with the new video/audio configure when switching happens. Other than that, I don't see a problem for now.
Re #50:

jrummell: Thanks for the links. I confirmed that this is indeed what we need.

Just one caveat. We need to run the test over https. Otherwise the test fails immediately with the following message: 

assert_unreached: NotSupportedError: Only secure origins are allowed (see: https://goo.gl/Y0ZkNV). Reached unreachable code

So the updated test links are:

https://w3c-test.org/encrypted-media/clearkey-mp4-playback-temporary-clear-encrypted.html
https://w3c-test.org/encrypted-media/clearkey-mp4-playback-temporary-encrypted-clear.html
https://w3c-test.org/encrypted-media/drm-mp4-playback-temporary-clear-encrypted.html
https://w3c-test.org/encrypted-media/drm-mp4-playback-temporary-encrypted-clear.html
Joey: As I am working on the implementation, does it make sense to also push this into the next version of the EME spec? e.g. the discussion at

https://github.com/w3c/encrypted-media/issues/251
I honestly have not given much thought to the spec.  I'm more concerned with the implementation right now than the spec, since this restriction in Chrome has been causing problems for our partners for a long time.  Thank you so much for working on it!
#56 - so will you give some thought to the specification? This is a browser, it is not only meant for your partners and it should be built on open standards, not on reverse-engineered ones. This can hurt the web.
Sorry, what I meant to say was that I'm really ill-prepared to comment on the spec at this time.  I need to make time to catch up on the spec conversations that have already happened.

I completely agree with you that browsers should be built on open standards and that the web is for everyone.  Open standards are critical to the web, and Google has a deep commitment to the open web.  I apologize for giving you the wrong impression.

Please note that the change we are discussing in Chrome:
 1. puts Chrome's implementation at parity with other browser implementations
 2. brings Chrome into conformance with existing W3C spec tests for EME

Therefore I think this change moves Chrome in the right direction with regard to the spec.

I will get caught up on the conversations and the relevant spec text.  If EME v1 needs to be clarified, I'll try to propose a specific clarification.  If this is an area that requires more detailed specification, perhaps this will become part of EME v2.
Gracias.
Labels: -M-57 M-58
Sorry I couldn't land the CL to enable transitioning from encrypted to clear streams in M57 and at this point I feel it's a bit late for a merge.

I'll try to land all CLs in M58, including enabling the switch both ways.

With that, update the milestone to M58. Let me know if you have any concerns. Sorry again about the delay.


I understand.  Thank you very much for working on this change.
Project Member

Comment 62 by bugdroid1@chromium.org, Feb 17 2017

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

commit 65c23034a1deaa9b221ef98928925bee36304fd6
Author: xhwang <xhwang@chromium.org>
Date: Fri Feb 17 04:37:42 2017

media: Allow config change between clear and encrypted streams

In the demuxer, we allow DecryptConfig change upon config change,
including but not limited to:
- switching between clear and encrypted
- encryption scheme change

Media Renderer implementation should support such changes. The detailed
requirement from the spec's perspective is tracked at:
https://github.com/w3c/encrypted-media/issues/251

Currently the default media Renderer (RendererImpl) supports switching
from encrypted to clear, because:
- Decrypt-and-decode mode: Decrypting{Audio|Video}Decoder supports clear
  buffer.
- Decrypt-only mode: DecryptingDemuxerStream supports clear buffer.

However, switching from clear to encrypted is not supported in
RendererImpl, because the clear decoder doesn't support decryption. This
will be fixed in a later CL.

BUG= 597443 
TEST=Updated pipeline_integration_tests.

Review-Url: https://codereview.chromium.org/2543623003
Cr-Commit-Position: refs/heads/master@{#451212}

[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/chrome/browser/media/media_browsertest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/chrome/browser/media/media_browsertest.h
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/content/browser/media/media_browsertest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/content/browser/media/media_browsertest.h
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/content/browser/media/media_source_browsertest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/content/renderer/media/cdm/ppapi_decryptor.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/content/renderer/pepper/content_decryptor_delegate.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/decoder_stream.h
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/decrypting_audio_decoder.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/decrypting_audio_decoder_unittest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/decrypting_demuxer_stream.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/decrypting_demuxer_stream_unittest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/decrypting_video_decoder.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/decrypting_video_decoder_unittest.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/filters/source_buffer_stream.cc
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/test/data/eme_player.html
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/test/data/eme_player_js/globals.js
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/test/data/eme_player_js/test_config.js
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/test/data/mse_config_change.html
[modify] https://crrev.com/65c23034a1deaa9b221ef98928925bee36304fd6/media/test/pipeline_integration_test.cc

Project Member

Comment 63 by bugdroid1@chromium.org, Feb 23 2017

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

commit 4f6b7bc7f2da4ae1813936fb341d4ed9ecbaeee8
Author: xhwang <xhwang@chromium.org>
Date: Thu Feb 23 01:19:52 2017

media: Add DecoderConfigType in DecoderStreamTraits

Add a function in DecoderStreamTraits to return the corresponding
DecoderConfigType, so that we can pass in the config to
Decoder::Initialize() instead of the whole stream. This is a trivial
refactoring which makes sense by itself since decoders should not talk
to DemuxerStream directly. Also, this will make supporting switch from
clear to encrypted stream easier (in a later CL).

BUG= 597443 
TEST=No functionality change.

Review-Url: https://codereview.chromium.org/2712843002
Cr-Commit-Position: refs/heads/master@{#452325}

[modify] https://crrev.com/4f6b7bc7f2da4ae1813936fb341d4ed9ecbaeee8/media/filters/decoder_selector.cc
[modify] https://crrev.com/4f6b7bc7f2da4ae1813936fb341d4ed9ecbaeee8/media/filters/decoder_stream.cc
[modify] https://crrev.com/4f6b7bc7f2da4ae1813936fb341d4ed9ecbaeee8/media/filters/decoder_stream_traits.cc
[modify] https://crrev.com/4f6b7bc7f2da4ae1813936fb341d4ed9ecbaeee8/media/filters/decoder_stream_traits.h

Project Member

Comment 64 by bugdroid1@chromium.org, Feb 24 2017

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

commit b2827d2c7e7fb65a6936bedb3b08409448e4d737
Author: xhwang <xhwang@chromium.org>
Date: Fri Feb 24 22:57:33 2017

media: Prefer decrypting pipeline when CDM is attached

In DecoderSelector, if a CDM is attached, always try the decrypting
pipeline first (decoders that support encrypted streams, or decrypting
demuxer stream plus regular decoders), so that the pipeline can handle
encrytped streams later.

BUG= 597443 
TEST=New tests enabled.

Review-Url: https://codereview.chromium.org/2701203003
Cr-Commit-Position: refs/heads/master@{#452975}

[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/base/audio_decoder_config.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/base/audio_decoder_config.h
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/base/video_decoder_config.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/base/video_decoder_config.h
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/audio_decoder_selector_unittest.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/decoder_selector.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/decoder_selector.h
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/decoder_stream.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/decrypting_audio_decoder.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/decrypting_audio_decoder_unittest.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/decrypting_demuxer_stream.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/decrypting_video_decoder.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/video_decoder_selector_unittest.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/filters/video_frame_stream_unittest.cc
[modify] https://crrev.com/b2827d2c7e7fb65a6936bedb3b08409448e4d737/media/test/pipeline_integration_test.cc

Cc: s...@chromium.org
dougsteed / slan: The Chrome side changes have been landed. 

I also made a pull request about the proposed spec change around this:
https://github.com/w3c/encrypted-media/pull/374/commits/04eeb40a1680cec471ec54996ee65589337799f2

In summary, "if MediaKeys is set before playback starts, the implementation **MUST** support switching between clear and encrypted streams".

In Chrome, if a CDM is attached before play, MojoRenderer (and CastRenderer) should get a SetCdm() call before Initialize(), which is the signal that there might be encrypted streaming coming (even thought the config in Initialize() might be clear). CastRenderer should be prepared to handled encrypted configs coming later, e.g. initialize the decrypting pipeline using the CDM.

I know ChormeCast has some internal changes to handle some switching cases. But we need to make sure it handles the switching both ways. We can file a separate bug if there are extra work to do.
Status: Fixed (was: Started)
Project Member

Comment 67 by bugdroid1@chromium.org, Oct 25

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

commit f085b7dd3bd8604654b5ca44fb037900d65c7b57
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Thu Oct 25 19:32:50 2018

media: Fix ClearThenEncrypted_MP4 pipeline integration test

Originally we were using a audio/video file but only specified a
video-only mime type, causing immediate parsing error in the demuxer.
This CL changes to use a video-only file to avoid this issue.

The test is also cleaned up and updated so now it passes.

Bug:  597443 
Test: Enable a disabled test.
Change-Id: I415ae8aa1ffcfb60be0d736660e45765e1c64d2b
Reviewed-on: https://chromium-review.googlesource.com/c/1296839
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602829}
[modify] https://crrev.com/f085b7dd3bd8604654b5ca44fb037900d65c7b57/media/test/pipeline_integration_test.cc

Sign in to add a comment