media: Allow config change between clear and encrypted streams

media/filters/decrypting_demuxer_stream.cc

Index: media/filters/decrypting_demuxer_stream.cc
diff --git a/media/filters/decrypting_demuxer_stream.cc b/media/filters/decrypting_demuxer_stream.cc
index 00e70e22a1692e3f29817ba9a337b2f4de017926..98d64934cbf2f3161b27687855e3031a749c4503 100644
--- a/media/filters/decrypting_demuxer_stream.cc
+++ b/media/filters/decrypting_demuxer_stream.cc
@@ -17,13 +17,11 @@
 
 namespace media {
 
-static bool IsStreamValidAndEncrypted(DemuxerStream* stream) {
+static bool IsStreamValid(DemuxerStream* stream) {
   return ((stream->type() == DemuxerStream::AUDIO &&
-           stream->audio_decoder_config().IsValidConfig() &&
-           stream->audio_decoder_config().is_encrypted()) ||
+           stream->audio_decoder_config().IsValidConfig()) ||
           (stream->type() == DemuxerStream::VIDEO &&
-           stream->video_decoder_config().IsValidConfig() &&
-           stream->video_decoder_config().is_encrypted()));
+           stream->video_decoder_config().IsValidConfig()));
 }
 
 DecryptingDemuxerStream::DecryptingDemuxerStream(
@@ -234,9 +232,15 @@ void DecryptingDemuxerStream::DecryptBuffer(
     return;
   }
 
-  DCHECK(buffer->decrypt_config());
+  if (!buffer->decrypt_config()) {

[ddorwin, 2016/12/05 23:53:04]

Why is this different from the block at 242? I would think we would prepend
"!buffer->decrypt_config() ||" to that condition.

It's good if we can avoid the copy, but it's not clear why. There hsould
probably be an explanation.

[xhwang, 2016/12/16 20:12:28]

I agree this is a bit odd. In the block at 242, we do a copy because there's no
way to easily flip the "is_encrypted" flag. Now is_encrypted() becomes a
method... I am not sure whether we can simply pass through as we do here. I'll
add a TODO and figure this out in a separate CL.

See previous discussion here:

https://bugs.chromium.org/p/chromium/issues/detail?id=171785#c6

+    DVLOG(2) << "DoDecryptBuffer() - clear buffer in clear stream.";
+    state_ = kIdle;
+    base::ResetAndReturn(&read_cb_).Run(kOk, buffer);

[ddorwin, 2016/12/05 23:53:04]

Should we report |status| instead like 231?

[xhwang, 2016/12/16 20:12:28]

|status| must be kOk here. Added a DCHECK and fixed 231 to make it clear.

+    return;
+  }
+
   if (!buffer->decrypt_config()->is_encrypted()) {
-    DVLOG(2) << "DoDecryptBuffer() - clear buffer.";
+    DVLOG(2) << "DoDecryptBuffer() - clear buffer in encrypted stream.";
     scoped_refptr<DecoderBuffer> decrypted = DecoderBuffer::CopyFrom(
         buffer->data(), buffer->data_size());
     decrypted->set_timestamp(buffer->timestamp());
@@ -362,9 +366,8 @@ Decryptor::StreamType DecryptingDemuxerStream::GetDecryptorStreamType() const {
 }
 
 void DecryptingDemuxerStream::InitializeDecoderConfig() {
-  // The decoder selector or upstream demuxer make sure the stream is valid and
-  // potentially encrypted.
-  DCHECK(IsStreamValidAndEncrypted(demuxer_stream_));
+  // The decoder selector or upstream demuxer make sure the stream is valid.
+  DCHECK(IsStreamValid(demuxer_stream_));
 
   switch (demuxer_stream_->type()) {
     case AUDIO: {