media: Allow config change between clear and encrypted streams

media/filters/decrypting_demuxer_stream.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/decrypting_demuxer_stream.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/media_log.h"
 #include "media/base/media_util.h"
 
 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(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     const scoped_refptr<MediaLog>& media_log,
     const base::Closure& waiting_for_decryption_key_cb)
     : task_runner_(task_runner),
       media_log_(media_log),
       state_(kUninitialized),
       waiting_for_decryption_key_cb_(waiting_for_decryption_key_cb),
@@ skipping 190 matching lines @@
     return;
   }
 
   if (buffer->end_of_stream()) {
     DVLOG(2) << "DoDecryptBuffer() - EOS buffer.";
     state_ = kIdle;
     base::ResetAndReturn(&read_cb_).Run(status, buffer);
     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());
     decrypted->set_duration(buffer->duration());
     if (buffer->is_key_frame())
       decrypted->set_is_key_frame(true);
 
     state_ = kIdle;
     base::ResetAndReturn(&read_cb_).Run(kOk, decrypted);
     return;
@@ skipping 105 matching lines @@
 
 Decryptor::StreamType DecryptingDemuxerStream::GetDecryptorStreamType() const {
   if (demuxer_stream_->type() == AUDIO)
     return Decryptor::kAudio;
 
   DCHECK_EQ(demuxer_stream_->type(), VIDEO);
   return Decryptor::kVideo;
 }
 
 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: {
       AudioDecoderConfig input_audio_config =
           demuxer_stream_->audio_decoder_config();
       audio_config_.Initialize(
           input_audio_config.codec(), input_audio_config.sample_format(),
           input_audio_config.channel_layout(),
           input_audio_config.samples_per_second(),
           input_audio_config.extra_data(), Unencrypted(),
@@ skipping 13 matching lines @@
       break;
     }
 
     default:
       NOTREACHED();
       return;
   }
 }
 
 }  // namespace media