Encrypted Media: Support Audio Decrypt-Only.

media/base/decryptor.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_DECRYPTOR_H_
 #define MEDIA_BASE_DECRYPTOR_H_
 
 #include <list>
 #include <string>
 
@@ skipping 186 matching lines @@
   // This method can be called any time after Initialize{Audio|Video}Decoder()
   // has been called (with the correct stream type).
   // After this operation, the decoder is set to an uninitialized state.
   // The decoder can be reinitialized after it is uninitialized.
   virtual void DeinitializeDecoder(StreamType stream_type) = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(Decryptor);
 };
 
+// Callback to notify decryptor creation.

[xhwang, 2012/12/11 02:27:40]

I moved this block here since it's duplicated in a lot of places. Also I feel
the name is too long... How about SetDecryptorCB and RegisterSetDecryptorCB?

[ddorwin, 2012/12/11 05:13:34]

I agree about the length (plus the lack of meaning).
Do we name CBs based on the function name or what they signal? Would
DecryptorCreatedCB work?

[xhwang, 2012/12/12 23:43:28]

Renamed.

+typedef base::Callback<void(Decryptor*)> DecryptorNotificationCB;
+// Callback to request/cancel decryptor creation notification.

[ddorwin, 2012/12/11 05:13:34]

"...notification of decryptor creation."

[xhwang, 2012/12/12 23:43:28]

Done.

+// Calling this callback with a non-null callback registers decryptor creation
+// notification. When the decryptor is created, notification will be sent
+// through the provided callback.
+// Calling this callback with a null callback cancels previously registered
+// decryptor creation notification. Any previously provided callback will be
+// fired immediately with NULL.
+typedef base::Callback<void(const DecryptorNotificationCB&)>

[ddorwin, 2012/12/11 05:13:34]

This is a callback for a callback? Seems weird. Maybe this is why naming is so
hard. See also comment about naming of the actual function in wmpi.cc. Maybe
fixing that will help.

[xhwang, 2012/12/11 19:43:04]

The original naming (proposed by fischman) actually looks good to me, except
it's too long.

We don't care if the decryptor is created, or how the decryptor will be used.
It's all about notifying the requester when a decryptor is available. That said,
I like the word "Register" too. How about DecryptorReadyCB and
RegisterDecryptorReadyCB?

[scherkus (not reviewing), 2012/12/11 20:52:15]

Well this is actually setting / unsetting an observer callback. In other words:

class DecryptorCreator {
  public void SetObserver(const DecryptorCB& cb) = 0;
};

I kind of like the word "set" as it doesn't make you think you can register
multiple callbacks.

How about:
DecryptorCreatedCB
SetDecryptorCreatedCB


... alternatively perhaps it would be clearer w/o callbacks and using something
more old fashioned like observer_list.h and C++ interfaces

[ddorwin, 2012/12/12 00:30:25]

Either of these SGTM. Combining scherkus's and xhwang's comments, we end up
with:
DecryptorReadyCB
SetDecryptorReadyCB

I think we should avoid pulling in new interfaces to this CL. If we want to
refactor (other than naming), let's do it in a separate CL.

[xhwang, 2012/12/12 23:43:28]

Done.

+    RequestDecryptorNotificationCB;
+
 }  // namespace media
 
 #endif  // MEDIA_BASE_DECRYPTOR_H_