Encrypted Media: Support Audio Decrypt-Only.

media/base/decryptor.h

Index: media/base/decryptor.h
diff --git a/media/base/decryptor.h b/media/base/decryptor.h
index 87c275d7a845ea7d4447d7dcefac4bdbb61fab25..1a19349eaeaae2bdfd61bf996695af1e6af30304 100644
--- a/media/base/decryptor.h
+++ b/media/base/decryptor.h
@@ -204,6 +204,18 @@ class MEDIA_EXPORT Decryptor {
   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_