media config: expand is_encrypted to a struct.

media/base/audio_decoder_config.h

 // 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.
 
 #ifndef MEDIA_BASE_AUDIO_DECODER_CONFIG_H_
 #define MEDIA_BASE_AUDIO_DECODER_CONFIG_H_
 
 #include <stdint.h>
 
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "media/base/audio_codecs.h"
 #include "media/base/channel_layout.h"
+#include "media/base/encryption_scheme.h"
 #include "media/base/media_export.h"
 #include "media/base/sample_format.h"
 
 namespace media {
 
 // TODO(dalecurtis): FFmpeg API uses |bytes_per_channel| instead of
 // |bits_per_channel|, we should switch over since bits are generally confusing
 // to work with.
 class MEDIA_EXPORT AudioDecoderConfig {
  public:
   // Constructs an uninitialized object. Clients should call Initialize() with
   // appropriate values before using.
   AudioDecoderConfig();
 
   // Constructs an initialized object.
   AudioDecoderConfig(AudioCodec codec,
                      SampleFormat sample_format,
                      ChannelLayout channel_layout,
                      int samples_per_second,
                      const std::vector<uint8_t>& extra_data,
-                     bool is_encrypted);
+                     const EncryptionScheme& encryption_scheme);
 
   AudioDecoderConfig(const AudioDecoderConfig& other);
 
   ~AudioDecoderConfig();
 
   // Resets the internal state of this object. |codec_delay| is in frames.
   void Initialize(AudioCodec codec,
                   SampleFormat sample_format,
                   ChannelLayout channel_layout,
                   int samples_per_second,
                   const std::vector<uint8_t>& extra_data,
-                  bool is_encrypted,
+                  const EncryptionScheme& encryption_scheme,
                   base::TimeDelta seek_preroll,
                   int codec_delay);
 
   // Returns true if this object has appropriate configuration values, false
   // otherwise.
   bool IsValidConfig() const;
 
   // Returns true if all fields in |config| match this config.
   // Note: The contents of |extra_data_| are compared not the raw pointers.
   bool Matches(const AudioDecoderConfig& config) const;
@@ skipping 12 matching lines @@
   base::TimeDelta seek_preroll() const { return seek_preroll_; }
   int codec_delay() const { return codec_delay_; }
 
   // Optional byte data required to initialize audio decoders such as Vorbis
   // codebooks.
   const std::vector<uint8_t>& extra_data() const { return extra_data_; }
 
   // Whether the audio stream is potentially encrypted.
   // Note that in a potentially encrypted audio stream, individual buffers
   // can be encrypted or not encrypted.
-  bool is_encrypted() const { return is_encrypted_; }
+  bool is_encrypted() const;

[ddorwin, 2016/03/02 23:24:05]

From PS13: I guess we should look for places where we expect it to be true in
tests and then check the other members in those cases.

[dougsteed, 2016/03/03 04:45:00]

OK, I'll scan through all instances with that in mind ....
... looked through all occurrences of is_encrypted() applied to
{Audio|Video}DecoderConfig.

Most of them are essentially asserting that it is not encrypted. I changed
media/filters/source_buffer_stream.cc to check the whole encryption_scheme
rather than just is_encrypted.
A few of the call sites are using is_encrypted() to evaluate a higher-level
notion of "IsEncrypted". I didn't follow all those paths.
The ones that I would like your input on are in: PpapiDecryptor,
DecryptingVideoDecoder, VideoDecodeAccelerator.

[ddorwin, 2016/03/03 18:38:04]

Thanks.

Those should three be fine since we only support CTR no pattern there. I was
specifically referring to tests. If there were any tests that expected true,
then we should check the mode and pattern (almost certainly CTR and none,
respectively).

[dougsteed, 2016/03/04 19:07:29]

In many cases the test uses a boolean to decide whether it should be configured
to use encryption, and then uses an EXPECT with the boolean is_encrypted to
verify whether that was done correctly. It doesn't seem to be testing much
outside the test itself. I didn't see any where I thought it would be useful to
verify the mode. 

Please take a quick peek yourself and see if you agree.

+
+  // Encryption scheme used for encrypted buffers.
+  const EncryptionScheme& encryption_scheme() const {
+    return encryption_scheme_;
+  }
 
  private:
   AudioCodec codec_;
   SampleFormat sample_format_;
   int bytes_per_channel_;
   ChannelLayout channel_layout_;
   int samples_per_second_;
   int bytes_per_frame_;
   std::vector<uint8_t> extra_data_;
-  bool is_encrypted_;
+  EncryptionScheme encryption_scheme_;
 
   // |seek_preroll_| is the duration of the data that the decoder must decode
   // before the decoded data is valid.
   base::TimeDelta seek_preroll_;
 
   // |codec_delay_| is the number of frames the decoder should discard before
   // returning decoded data.  This value can include both decoder delay as well
   // as padding added during encoding.
   int codec_delay_;
 
   // Not using DISALLOW_COPY_AND_ASSIGN here intentionally to allow the compiler
   // generated copy constructor and assignment operator. Since the extra data is
   // typically small, the performance impact is minimal.
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_AUDIO_DECODER_CONFIG_H_