Update PluginInstance for audio support for content decryption.

webkit/plugins/ppapi/ppapi_plugin_instance.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 "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
 #include "base/time.h"
 #include "base/utf_offset_string_conversions.h"
 #include "base/utf_string_conversions.h"
 // TODO(xhwang): Move media specific code out of this class.
 #include "media/base/audio_decoder_config.h"
+#include "media/base/channel_layout.h"
+#include "media/base/data_buffer.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/decryptor_client.h"
 #include "media/base/video_decoder_config.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
 #include "ppapi/c/dev/ppb_find_dev.h"
 #include "ppapi/c/dev/ppb_zoom_dev.h"
 #include "ppapi/c/dev/ppp_find_dev.h"
 #include "ppapi/c/dev/ppp_selection_dev.h"
 #include "ppapi/c/dev/ppp_text_input_dev.h"
@@ skipping 355 matching lines @@
   for (uint32_t i = 0; i < block_info->num_subsamples; ++i) {
     block_info->subsamples[i].clear_bytes =
         decrypt_config->subsamples()[i].clear_bytes;
     block_info->subsamples[i].cipher_bytes =
         decrypt_config->subsamples()[i].cypher_bytes;
   }
 
   return true;
 }
 
+bool MakeAudioFrames(PP_Resource audio_frames,

[xhwang, 2012/10/19 17:07:33]

probably need comments here.

[Tom Finegan, 2012/10/19 22:53:52]

Agreed, but I don't think it needs anything too big. How about:

Deserializes audio data stored in |audio_frames| into individual audio buffers
in |frames|. Returns true upon success.

I don't know.... writing that makes me think the function should be named
DeserializeAudioFrames(). WDYT?

[ddorwin, 2012/10/20 00:14:36]

Especially since the out parameter type is "buffers". :)

[xhwang, 2012/10/21 19:05:06]

Done.

[xhwang, 2012/10/21 19:05:06]

Hmm, maybe we should rename media::Decryptor::AudioBuffers to
media::Decryptor::AudioFrames ?

[ddorwin, 2012/10/22 17:47:11]

SGTM (in another CL).

+                     media::Decryptor::AudioBuffers* frames) {
+  DCHECK(frames);
+  EnterResourceNoLock<PPB_Buffer_API> enter(audio_frames, true);
+  if (!enter.succeeded())
+    return false;
+
+  BufferAutoMapper mapper(enter.object());
+  if (!mapper.data() || !mapper.size())
+    return false;
+
+  const uint8* cur = static_cast<uint8*>(mapper.data());
+  int bytes_left = mapper.size();
+
+  do {
+    if (bytes_left < 16)

[ddorwin, 2012/10/20 00:14:36]

why?
16 is a magic number here and below. 8 too. sizeof()?

[xhwang, 2012/10/21 19:05:06]

Done.

+      return false;
+
+    int64 timestamp = *(reinterpret_cast<const int64*>(cur));
+    cur += 8;
+    int64 frame_size = *(reinterpret_cast<const int64*>(cur));
+    cur += 8;
+    bytes_left -= 16;

[ddorwin, 2012/10/20 00:14:36]

(8 + 8) would be clearer, esp. when using sizeof().

[xhwang, 2012/10/21 19:05:06]

Done.

+
+    if (frame_size < 0 || bytes_left < frame_size)

[ddorwin, 2012/10/20 00:14:36]

DCHECK? Is this a bug if we see it?

[xhwang, 2012/10/21 19:05:06]

As I said, I don't trust CDMs. The renderer should not crash if the CDM returns
garbage. WDYT?

[ddorwin, 2012/10/22 17:47:11]

A DCHECK just tells us something is wrong in a debug build. So we can
investigate. We should still have the error handling for prod builds.

+      return false;
+
+    scoped_refptr<media::DataBuffer> frame(new media::DataBuffer(frame_size));
+    if (frame_size > 0) {
+      memcpy(frame->GetWritableData(), cur, frame_size);
+      frame->SetTimestamp(base::TimeDelta::FromMicroseconds(timestamp));
+      cur += frame_size;
+      bytes_left -= frame_size;
+    }
+    frames->push_back(frame);
+  }
+  while (bytes_left > 0);

[Tom Finegan, 2012/10/19 22:53:52]

Move the while up on the same line with the closing brace for the do.

[xhwang, 2012/10/21 19:05:06]

Done.

+
+  return true;
+}
+
 PP_AudioCodec MediaAudioCodecToPpAudioCodec(media::AudioCodec codec) {
   if (codec == media::kCodecVorbis)
     return PP_AUDIOCODEC_VORBIS;
 
   return PP_AUDIOCODEC_UNKNOWN;
 }
 
 PP_VideoCodec MediaVideoCodecToPpVideoCodec(media::VideoCodec codec) {
   if (codec == media::kCodecVP8)
     return PP_VIDEOCODEC_VP8;
@@ skipping 88 matching lines @@
       text_input_type_(kPluginDefaultTextInputType),
       text_input_caret_(0, 0, 0, 0),
       text_input_caret_bounds_(0, 0, 0, 0),
       text_input_caret_set_(false),
       selection_caret_(0),
       selection_anchor_(0),
       pending_user_gesture_(0.0),
       flash_impl_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       decryptor_client_(NULL),
       next_decryption_request_id_(1),
+      pending_audio_decrypt_request_id_(0),
+      pending_video_decrypt_request_id_(0),
       pending_audio_decoder_init_request_id_(0),
       pending_video_decoder_init_request_id_(0),
+      pending_audio_decode_request_id_(0),
       pending_video_decode_request_id_(0) {
   pp_instance_ = HostGlobals::Get()->AddInstance(this);
 
   memset(&current_print_settings_, 0, sizeof(current_print_settings_));
   DCHECK(delegate);
   module_->InstanceCreated(this);
   delegate_->InstanceCreated(this);
   message_channel_.reset(new MessageChannel(this));
 
   view_data_.is_page_visible = delegate->IsPageVisible();
@@ skipping 986 matching lines @@
   PP_PrivatePageTransformType transform_type =
       type == WebPlugin::RotationType90Clockwise ?
       PP_PRIVATEPAGETRANSFORMTYPE_ROTATE_90_CW :
       PP_PRIVATEPAGETRANSFORMTYPE_ROTATE_90_CCW;
   plugin_pdf_interface_->Transform(pp_instance(), transform_type);
   // NOTE: plugin instance may have been deleted.
 }
 
 void PluginInstance::set_decrypt_client(
     media::DecryptorClient* decryptor_client) {
-  DCHECK(decryptor_client);
   decryptor_client_ = decryptor_client;
 }
 
 bool PluginInstance::GenerateKeyRequest(const std::string& key_system,
                                         const std::string& init_data) {
   if (!LoadContentDecryptorInterface())
     return false;
   if (key_system.empty())
     return false;
 
@@ skipping 31 matching lines @@
 
 bool PluginInstance::CancelKeyRequest(const std::string& session_id) {
   if (!LoadContentDecryptorInterface())
     return false;
   plugin_decryption_interface_->CancelKeyRequest(
       pp_instance(),
       StringVar::StringToPPVar(session_id));
   return true;
 }
 
+// TODO(xhwang): Remove duplication of code in Decrypt(),
+// DecryptAndDecodeAudio() and DecryptAndDecodeVideo().

[Tom Finegan, 2012/10/19 22:53:52]

We should do this when we refactor and add the delegate.

[xhwang, 2012/10/21 19:05:06]

Agreed.

 bool PluginInstance::Decrypt(
+    media::Decryptor::StreamType stream_type,
     const scoped_refptr<media::DecoderBuffer>& encrypted_buffer,
     const media::Decryptor::DecryptCB& decrypt_cb) {
-  DVLOG(3) << "Decrypt()";
+  DVLOG(3) << "Decrypt() - stream_type: " << stream_type;
   if (!LoadContentDecryptorInterface())
     return false;
 
   ScopedPPResource encrypted_resource(
       ScopedPPResource::PassRef(),
       MakeBufferResource(pp_instance(),
                          encrypted_buffer->GetData(),
                          encrypted_buffer->GetDataSize()));
   if (!encrypted_resource.get())
     return false;
 
   const uint32_t request_id = next_decryption_request_id_++;
   DVLOG(2) << "Decrypt() - request_id " << request_id;
 
   PP_EncryptedBlockInfo block_info;
   DCHECK(encrypted_buffer->GetDecryptConfig());
   if (!MakeEncryptedBlockInfo(encrypted_buffer->GetDecryptConfig(),
                               encrypted_buffer->GetTimestamp().InMicroseconds(),
                               request_id,
                               &block_info)) {
     return false;
   }
 
-  DCHECK(!ContainsKey(pending_decryption_cbs_, request_id));
-  pending_decryption_cbs_.insert(std::make_pair(request_id, decrypt_cb));
+  uint32_t& pending_decrypt_request_id =

[ddorwin, 2012/10/20 00:14:36]

DCHECK stream_type,
Would a switch statement be equiv or less code and less confusing?

[xhwang, 2012/10/21 19:05:06]

Done.

+      (stream_type == media::Decryptor::kAudio) ?
+      pending_audio_decrypt_request_id_ : pending_video_decrypt_request_id_;
+  media::Decryptor::DecryptCB& pending_decrypt_cb =
+      (stream_type == media::Decryptor::kAudio) ?
+      pending_audio_decrypt_cb_ : pending_video_decrypt_cb_;
+
+  // There is only one pending decrypt request at any time per stream. This is
+  // enforced by the media pipeline.
+  DCHECK_EQ(pending_decrypt_request_id, 0u);
+  DCHECK(pending_decrypt_cb.is_null());
+  pending_decrypt_request_id = request_id;
+  pending_decrypt_cb = decrypt_cb;
 
   plugin_decryption_interface_->Decrypt(pp_instance(),
                                         encrypted_resource,
                                         &block_info);
   return true;
 }
 
+bool PluginInstance::CancelDecrypt(media::Decryptor::StreamType stream_type) {

[ddorwin, 2012/10/20 00:14:36]

Is it easy for the client to know whether to call CancelDecrypt vs.
DeinitializeDecoder (and thus CancelDecode)? Do we really need separate
functions or could we just cancel whatever we are doing for audio/video based on
stream_type? If we can't do that, should we at least standardize on Cancel or
Deinitialize?

[xhwang, 2012/10/21 19:05:06]

I agree it's confusing.

For decoder, we have ResetDecoder and DeinitializeDecoder. For decryptor, we
choose "Cancel" because the decryptor is stateless, so we don't really need to
reset the decryptor other than canceling the cb. Also, we call it
Cancel*Decrypt* so it's clear that we are cancelling the decryption operation
scheduled by Decrypt(). 

It would be nice to consolidate CancelDecryptor and ResetDecoder. To do that,
first we need a proper name, e.g. Reset(stream_type). Not sure if it's clearer
for the caller though. Second, we need two extra variables to remember which
path (DO or D&D) the PluginInstance has chosen. 

One thing I don't like about consolidated functions for DO and D&D is that we
have to document and enforce in PI that DO and D&D methods can't live together.
For example, if Decrypt() is called, InitializeVideoDecoder() should not be
called. For now, we don't need to worry about this.

WDYT?

+  DVLOG(3) << "CancelDecrypt() - stream_type: " << stream_type;
+
+  media::Decryptor::DecryptCB decrypt_cb;
+  switch (stream_type) {
+    case media::Decryptor::kAudio:
+      pending_audio_decrypt_request_id_ = 0;
+      decrypt_cb = base::ResetAndReturn(&pending_audio_decrypt_cb_);

[ddorwin, 2012/10/20 00:14:36]

Does this work if pending_audio_decrypt_cb_ is null/empty?

[xhwang, 2012/10/21 19:05:06]

Yes. Line 1676 checks this condition.

+      break;
+    case media::Decryptor::kVideo:
+      pending_video_decrypt_request_id_ = 0;
+      decrypt_cb = base::ResetAndReturn(&pending_video_decrypt_cb_);
+      break;
+    default:
+      NOTREACHED();
+      return false;
+  }
+
+  if (!decrypt_cb.is_null())
+    decrypt_cb.Run(media::Decryptor::kSuccess, NULL);
+
+  return true;
+}
+
 bool PluginInstance::InitializeAudioDecoder(
     const media::AudioDecoderConfig& decoder_config,
     const media::Decryptor::DecoderInitCB& init_cb) {
   PP_AudioDecoderConfig pp_decoder_config;
   pp_decoder_config.channel_count =
       media::ChannelLayoutToChannelCount(decoder_config.channel_layout());
   pp_decoder_config.bits_per_channel = decoder_config.bits_per_channel();
   pp_decoder_config.samples_per_second = decoder_config.samples_per_second();
   pp_decoder_config.request_id = next_decryption_request_id_++;
 
@@ skipping 38 matching lines @@
   DCHECK(pending_video_decoder_init_cb_.is_null());
   pending_video_decoder_init_request_id_ = pp_decoder_config.request_id;
   pending_video_decoder_init_cb_ = init_cb;
 
   plugin_decryption_interface_->InitializeVideoDecoder(pp_instance(),
                                                        &pp_decoder_config,
                                                        extra_data_resource);
   return true;
 }
 
-bool PluginInstance::DeinitializeDecoder() {
+void PluginInstance::CancelDecode(media::Decryptor::StreamType stream_type) {
+  switch (stream_type) {
+    case media::Decryptor::kAudio:
+      pending_audio_decode_request_id_ = 0;
+      if (!pending_audio_decode_cb_.is_null())
+        base::ResetAndReturn(&pending_audio_decode_cb_).Run(
+            media::Decryptor::kSuccess, media::Decryptor::AudioBuffers());

[ddorwin, 2012/10/20 00:14:36]

odd that video passes NULL but this doesn't. I assume there is a reason.

[xhwang, 2012/10/21 19:05:06]

VideoDecodeCB takes scoped_refptr<VideoFrame>, so we use NULL.
AudioDecodeCB takes std::list<Buffer>, so we pass empty list.

+      break;
+    case media::Decryptor::kVideo:
+      pending_video_decode_request_id_ = 0;
+      if (!pending_video_decode_cb_.is_null())
+        base::ResetAndReturn(&pending_video_decode_cb_).Run(
+            media::Decryptor::kSuccess, NULL);
+      break;
+    default:
+      NOTREACHED();
+  }
+}
+
+bool PluginInstance::DeinitializeDecoder(
+    media::Decryptor::StreamType stream_type) {
   if (!LoadContentDecryptorInterface())
     return false;
 
+  CancelDecode(stream_type);
+
   // TODO(tomfinegan): Add decoder deinitialize request tracking, and get
   // stream type from media stack.
   plugin_decryption_interface_->DeinitializeDecoder(
       pp_instance(),
       PP_DECRYPTORSTREAMTYPE_VIDEO,
       0);
   return true;
 }
 
-bool PluginInstance::ResetDecoder() {
+bool PluginInstance::ResetDecoder(media::Decryptor::StreamType stream_type) {

[ddorwin, 2012/10/20 00:14:36]

Does just a Reset() for decrypt & D&D make sense?

[xhwang, 2012/10/21 19:05:06]

We can discuss about this. See above comment.

   if (!LoadContentDecryptorInterface())
     return false;
 
+  CancelDecode(stream_type);
+
   // TODO(tomfinegan): Add decoder reset request tracking, and get
   // stream type from media stack.
   plugin_decryption_interface_->ResetDecoder(pp_instance(),
                                              PP_DECRYPTORSTREAMTYPE_VIDEO,
                                              0);
   return true;
 }
 
-bool PluginInstance::DecryptAndDecode(
+bool PluginInstance::DecryptAndDecodeAudio(
+    const scoped_refptr<media::DecoderBuffer>& encrypted_buffer,
+    const media::Decryptor::AudioDecodeCB& audio_decode_cb) {
+  if (!LoadContentDecryptorInterface())
+    return false;
+
+  // If |encrypted_buffer| is end-of-stream buffer, GetData() and GetDataSize()
+  // return NULL and 0 respectively. In that case, we'll just create a 0
+  // resource.
+  ScopedPPResource encrypted_resource(
+      ScopedPPResource::PassRef(),
+      MakeBufferResource(pp_instance(),
+                         encrypted_buffer->GetData(),
+                         encrypted_buffer->GetDataSize()));
+  if (!encrypted_buffer->IsEndOfStream() && !encrypted_resource.get())
+    return false;
+
+  const uint32_t request_id = next_decryption_request_id_++;
+  DVLOG(2) << "DecryptAndDecode() - request_id " << request_id;

[Tom Finegan, 2012/10/19 22:53:52]

s/DecryptAndDecode/DecryptAndDecodeAudio/

(Here and below)

[xhwang, 2012/10/21 19:05:06]

Done.

+
+  PP_EncryptedBlockInfo block_info;
+  if (!MakeEncryptedBlockInfo(
+      encrypted_buffer->GetDecryptConfig(),
+      encrypted_buffer->GetTimestamp().InMicroseconds(),
+      request_id,
+      &block_info)) {
+    return false;
+  }
+
+  // There is only one pending audio decode request at any time. This is
+  // enforced by the media pipeline.
+  DCHECK_EQ(pending_audio_decode_request_id_, 0u);
+  DCHECK(pending_audio_decode_cb_.is_null());
+  pending_audio_decode_request_id_ = request_id;
+  pending_audio_decode_cb_ = audio_decode_cb;
+
+  plugin_decryption_interface_->DecryptAndDecode(pp_instance(),
+                                                 PP_DECRYPTORSTREAMTYPE_AUDIO,
+                                                 encrypted_resource,
+                                                 &block_info);
+  return true;
+}
+
+bool PluginInstance::DecryptAndDecodeVideo(
     const scoped_refptr<media::DecoderBuffer>& encrypted_buffer,
     const media::Decryptor::VideoDecodeCB& video_decode_cb) {
   if (!LoadContentDecryptorInterface())
     return false;
 
   // If |encrypted_buffer| is end-of-stream buffer, GetData() and GetDataSize()
   // return NULL and 0 respectively. In that case, we'll just create a 0
   // resource.
   ScopedPPResource encrypted_resource(
       ScopedPPResource::PassRef(),
@@ skipping 648 matching lines @@
 void PluginInstance::NeedKey(PP_Instance instance,
                              PP_Var key_system_var,
                              PP_Var session_id_var,
                              PP_Var init_data_var) {
   // TODO(tomfinegan): send the data to media stack.
 }
 
 void PluginInstance::KeyAdded(PP_Instance instance,
                               PP_Var key_system_var,
                               PP_Var session_id_var) {
+  if (!decryptor_client_)
+    return;
+
   StringVar* key_system_string = StringVar::FromPPVar(key_system_var);
   StringVar* session_id_string = StringVar::FromPPVar(session_id_var);
   if (!key_system_string || !session_id_string) {
     decryptor_client_->KeyError("", "", media::Decryptor::kUnknownError, 0);
     return;
   }
 
-  DCHECK(decryptor_client_);
   decryptor_client_->KeyAdded(key_system_string->value(),
                               session_id_string->value());
 }
 
 void PluginInstance::KeyMessage(PP_Instance instance,
                                 PP_Var key_system_var,
                                 PP_Var session_id_var,
                                 PP_Resource message_resource,
                                 PP_Var default_url_var) {
+  if (!decryptor_client_)
+    return;
+
   StringVar* key_system_string = StringVar::FromPPVar(key_system_var);
   StringVar* session_id_string = StringVar::FromPPVar(session_id_var);
   StringVar* default_url_string = StringVar::FromPPVar(default_url_var);
 
   if (!key_system_string || !session_id_string || !default_url_string) {
     decryptor_client_->KeyError("", "", media::Decryptor::kUnknownError, 0);
     return;
   }
 
   EnterResourceNoLock<PPB_Buffer_API> enter(message_resource, true);
   if (!enter.succeeded()) {
     decryptor_client_->KeyError(key_system_string->value(),
                                 session_id_string->value(),
                                 media::Decryptor::kUnknownError,
                                 0);
     return;
   }
 
   BufferAutoMapper mapper(enter.object());
   scoped_array<uint8> message_array(new uint8[mapper.size()]);
   if (mapper.data() && mapper.size())
     memcpy(message_array.get(), mapper.data(), mapper.size());
 
-  DCHECK(decryptor_client_);
   decryptor_client_->KeyMessage(key_system_string->value(),
                                 session_id_string->value(),
                                 message_array.Pass(),
                                 mapper.size(),
                                 default_url_string->value());
 }
 
 void PluginInstance::KeyError(PP_Instance instance,
                               PP_Var key_system_var,
                               PP_Var session_id_var,
                               int32_t media_error,
                               int32_t system_code) {
+  if (!decryptor_client_)
+    return;
+
   StringVar* key_system_string = StringVar::FromPPVar(key_system_var);
   StringVar* session_id_string = StringVar::FromPPVar(session_id_var);
   if (!key_system_string || !session_id_string) {
     decryptor_client_->KeyError("", "", media::Decryptor::kUnknownError, 0);
     return;
   }
 
-  DCHECK(decryptor_client_);
   decryptor_client_->KeyError(
       key_system_string->value(),
       session_id_string->value(),
       static_cast<media::Decryptor::KeyError>(media_error),
       system_code);
 }
 
 void PluginInstance::DecoderInitializeDone(PP_Instance instance,
                                            PP_DecryptorStreamType decoder_type,
                                            uint32_t request_id,
@@ skipping 33 matching lines @@
                                       uint32_t request_id) {
   // TODO(tomfinegan): Add decoder reset completion handling.
 }
 
 void PluginInstance::DeliverBlock(PP_Instance instance,
                                   PP_Resource decrypted_block,
                                   const PP_DecryptedBlockInfo* block_info) {
   DVLOG(2) << "DeliverBlock() - request_id: "
            << block_info->tracking_info.request_id;
   DCHECK(block_info);
-  DecryptionCBMap::iterator found = pending_decryption_cbs_.find(
-      block_info->tracking_info.request_id);
-  if (found == pending_decryption_cbs_.end())
+
+  // If the request ID is not valid or does not match what's saved, do nothing.
+  const uint32_t request_id = block_info->tracking_info.request_id;
+  if (request_id == 0)
     return;
-  media::Decryptor::DecryptCB decrypt_cb = found->second;
-  pending_decryption_cbs_.erase(found);
+
+  media::Decryptor::DecryptCB decrypt_cb;
+  if (request_id == pending_audio_decrypt_request_id_) {
+    DCHECK(!pending_audio_decrypt_cb_.is_null());
+    pending_audio_decrypt_request_id_ = 0;
+    decrypt_cb = base::ResetAndReturn(&pending_audio_decrypt_cb_);
+  } else if (request_id == pending_video_decrypt_request_id_) {
+    DCHECK(!pending_video_decrypt_cb_.is_null());
+    pending_video_decrypt_request_id_ = 0;
+    decrypt_cb = base::ResetAndReturn(&pending_video_decrypt_cb_);
+  } else {
+    DVLOG(1) << "DeliverBlock() - request_id "
+             << block_info->tracking_info.request_id << " no found";

[ddorwin, 2012/10/20 00:14:36]

s/no/not/

[ddorwin, 2012/10/20 00:14:36]

you already have request_id local var you can use.

[xhwang, 2012/10/21 19:05:06]

Done.

+    return;
+  }
 
   if (block_info->result == PP_DECRYPTRESULT_DECRYPT_NOKEY) {
     decrypt_cb.Run(media::Decryptor::kNoKey, NULL);
     return;
   }
 
   if (block_info->result != PP_DECRYPTRESULT_SUCCESS) {
     decrypt_cb.Run(media::Decryptor::kError, NULL);
     return;
   }
@@ skipping 99 matching lines @@
       decoded_frame.get());
 
   video_decode_cb.Run(media::Decryptor::kSuccess, decoded_frame);
 }
 
 void PluginInstance::DeliverSamples(PP_Instance instance,
                                     PP_Resource audio_frames,
                                     const PP_DecryptedBlockInfo* block_info) {
   DVLOG(2) << "DeliverSamples() - request_id: "
            << block_info->tracking_info.request_id;
-  // TODO(tomfinegan): To be implemented after completion of v0.1 of the
-  // EME/CDM work.
+  DCHECK(audio_frames);

[ddorwin, 2012/10/20 00:14:36]

decrypted_block can be null above but this can't? or just a missing DCHECK
above?

[xhwang, 2012/10/21 19:05:06]

Done.

+
+  // If the request ID is not valid or does not match what's saved, do nothing.
+  if (block_info->tracking_info.request_id == 0 ||
+      block_info->tracking_info.request_id !=
+          pending_audio_decode_request_id_) {
+    DCHECK(pending_audio_decode_cb_.is_null());
+    return;
+  }
+
+  DCHECK(!pending_audio_decode_cb_.is_null());
+  pending_audio_decode_request_id_ = 0;
+  media::Decryptor::AudioDecodeCB audio_decode_cb =
+      base::ResetAndReturn(&pending_audio_decode_cb_);
+
+  const media::Decryptor::AudioBuffers empty_frames;
+
+  if (block_info->result == PP_DECRYPTRESULT_DECRYPT_NOKEY) {
+    audio_decode_cb.Run(media::Decryptor::kNoKey, empty_frames);
+    return;
+  }
+
+  if (block_info->result != PP_DECRYPTRESULT_SUCCESS) {
+    audio_decode_cb.Run(media::Decryptor::kError, empty_frames);
+    return;
+  }
+
+  if (audio_frames == 0) {  // End-of-stream frame.

[xhwang, 2012/10/19 17:07:33]

We can remove this check if we all agree that EOS frame should always be a
zero-size buffer in audio_frames.

[Tom Finegan, 2012/10/19 22:53:52]

This isn't necessarily and end of stream frame. The decoder might need more data
to produce output.

It might be simpler to just give in and add PP_DECRYPTRESULT_NEEDMOREDATA.

[xhwang, 2012/10/21 19:05:06]

Yeah, done in another CL: 11234019

+    audio_decode_cb.Run(media::Decryptor::kSuccess, empty_frames);
+    return;
+  }
+
+  media::Decryptor::AudioBuffers audio_frame_list;
+  if (!MakeAudioFrames(audio_frames, &audio_frame_list)) {
+    audio_decode_cb.Run(media::Decryptor::kError, empty_frames);
+    return;
+  }
+
+  audio_decode_cb.Run(media::Decryptor::kSuccess, audio_frame_list);
 }
 
 void PluginInstance::NumberOfFindResultsChanged(PP_Instance instance,
                                                 int32_t total,
                                                 PP_Bool final_result) {
   DCHECK_NE(find_identifier_, -1);
   delegate_->NumberOfFindResultsChanged(find_identifier_, total,
                                         PP_ToBool(final_result));
 }
 
@@ skipping 369 matching lines @@
   screen_size_for_fullscreen_ = gfx::Size();
   WebElement element = container_->element();
   element.setAttribute(WebString::fromUTF8(kWidth), width_before_fullscreen_);
   element.setAttribute(WebString::fromUTF8(kHeight), height_before_fullscreen_);
   element.setAttribute(WebString::fromUTF8(kBorder), border_before_fullscreen_);
   element.setAttribute(WebString::fromUTF8(kStyle), style_before_fullscreen_);
 }
 
 }  // namespace ppapi
 }  // namespace webkit