MediaCodecPlayer implementation - stage 7 (DRM)

media/base/android/media_codec_player.h

Index: media/base/android/media_codec_player.h
diff --git a/media/base/android/media_codec_player.h b/media/base/android/media_codec_player.h
index 1bc025427a27d8c7c4640e0ab927314d4715006d..55ce0bebe2f17912683a649015fe856eca9730d0 100644
--- a/media/base/android/media_codec_player.h
+++ b/media/base/android/media_codec_player.h
@@ -11,6 +11,7 @@
 #include "base/threading/thread.h"
 #include "base/time/default_tick_clock.h"
 #include "media/base/android/demuxer_android.h"
+#include "media/base/android/media_drm_bridge.h"
 #include "media/base/android/media_player_android.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/media_export.h"
@@ -235,6 +236,8 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid,
     kStatePlaying,
     kStateStopping,
     kStateWaitingForSurface,
+    kStateWaitingForKey,
+    kStateWaitingForCrypto,

[xhwang, 2015/09/30 18:07:53]

nit: Crypto can mean a lot of things while MediaCrypto is a concrete Android API
object. Can we make it clearer to name this kStateWaitingForMediaCrypto

[Tima Vaisburd, 2015/09/30 23:00:50]

Done.

     kStateWaitingForSeek,
     kStateError,

[xhwang, 2015/09/30 18:07:53]

nit: We still prefer MACRO_STYLE to kConstantNaming style for enums:
https://www.chromium.org/developers/coding-style

[Tima Vaisburd, 2015/09/30 23:00:50]

I found the corresponding line. Thank you.
I have to say though that I have already changed the MACRO_STYLE to kConstStyle
everywhere by one reviewer request. If I have to change everywhere again, let it
be a separate CL.

[xhwang, 2015/10/01 00:04:39]

A separate CL will be great.

   };
@@ -242,6 +245,7 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid,
   enum StartStatus {
     kStartOk = 0,
     kStartBrowserSeekRequired,
+    kStartCryptoRequired,
     kStartFailed,
   };
 
@@ -283,6 +287,7 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid,
   void OnPrerollDone();
   void OnDecoderDrained(DemuxerStream::Type type);
   void OnStopDone(DemuxerStream::Type type);
+  void OnKeyRequired(DemuxerStream::Type type);

[xhwang, 2015/09/30 18:07:54]

nit: OnWaitingForDecryptionKey()?

[Tima Vaisburd, 2015/09/30 23:00:50]

Renamed to OnMissingKeyReported(). As I mentioned before, I feel easily confused
when using "waiting" in a name of an event, tried to make it something that
"happened".

[xhwang, 2015/10/01 00:04:40]

That makes sense. Can we kinda consolidate this name and the variable name
|key_request_posted_|?

[Tima Vaisburd, 2015/10/01 00:51:20]

s/key_request_posted_/missing_key_reported_/

   void OnError();
   void OnStarvation(DemuxerStream::Type stream_type);
   void OnTimeIntervalUpdate(DemuxerStream::Type stream_type,
@@ -294,6 +299,12 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid,
   void OnVideoCodecCreated();
   void OnVideoResolutionChanged(const gfx::Size& size);
 
+  // Callbacks from DRM

[xhwang, 2015/09/30 18:07:54]

nit: I would s/DRM/CDM :)

[Tima Vaisburd, 2015/09/30 23:00:50]

Done.

Mmm... does CDM stand for Content Decryption Module and DRM for Digital Right
Management?

[xhwang, 2015/10/01 00:04:39]

Yeah. In chromium/EME we don't say DRM. The only reason we have it in some
places is MediaDrm/MediaDrmBridge.

+  void OnMediaCryptoReady(MediaDrmBridge::JavaObjectPtr media_crypto,
+                          bool needs_protected_surface);
+  void OnKeyAdded();
+  void OnCdmUnset();
+
   // Operations called from the state machine.
   void SetState(PlayerState new_state);
   void SetPendingStart(bool need_to_start);
@@ -341,6 +352,7 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid,
   base::Closure request_resources_cb_;
   TimeUpdateCallback time_update_cb_;
   base::Closure completion_cb_;
+  base::Closure key_required_cb_;

[xhwang, 2015/09/30 18:07:54]

nit: This is more commonly called |waiting_for_decryption_key_cb|: 

https://code.google.com/p/chromium/codesearch#search/&q=waiting_for_decryptio...

This is also more consistent with the kStateWaitingForKey state.

[Tima Vaisburd, 2015/09/30 23:00:50]

Done.

   SeekDoneCallback seek_done_cb_;
   ErrorCallback error_cb_;
 
@@ -383,6 +395,20 @@ class MEDIA_EXPORT MediaCodecPlayer : public MediaPlayerAndroid,
   // For testing only.
   DecodersTimeCallback decoders_time_cb_;
 
+  // DRM
+  MediaDrmBridge::JavaObjectPtr media_crypto_;
+
+  MediaDrmBridge* drm_bridge_;
+  int cdm_registration_id_;
+
+  // The flag is set when the player receives the error from decoder that the
+  // decoder needs a new encryption key. Cleared on starting the playback.

[xhwang, 2015/09/30 18:07:53]

s/encryption/decryption

[Tima Vaisburd, 2015/09/30 23:00:50]

Done.

+  bool key_is_required_;

[xhwang, 2015/09/30 18:07:53]

What's the relation between this and state_ == kStateWaitingForKey?

[Tima Vaisburd, 2015/09/30 21:24:51]

My intention was to set the |key_is_required_| every time we receive a
corresponding error from decoder, but to enter the waiting state during the
restart process (I check it after Prefetch).

+
+  // The flag is set after the new encryption key is added to MediaDrm. Cleared
+  // on starting the playback.
+  bool key_is_added_;
+
   base::WeakPtr<MediaCodecPlayer> media_weak_this_;
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<MediaCodecPlayer> media_weak_factory_;