Redesign the copresence audio handlers.

components/copresence/handlers/audio/audio_directive_handler.cc

Index: components/copresence/handlers/audio/audio_directive_handler.cc
diff --git a/components/copresence/handlers/audio/audio_directive_handler.cc b/components/copresence/handlers/audio/audio_directive_handler.cc
index 3d2e7c65aac69b8150c564c92550d5f20a486639..04bd9822ac8a392a21c9353d6b8104065234e303 100644
--- a/components/copresence/handlers/audio/audio_directive_handler.cc
+++ b/components/copresence/handlers/audio/audio_directive_handler.cc
@@ -7,67 +7,27 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/strings/string_util.h"
 #include "base/time/time.h"
-#include "components/copresence/mediums/audio/audio_player.h"
-#include "components/copresence/mediums/audio/audio_recorder.h"
 #include "components/copresence/proto/data.pb.h"
+#include "components/copresence/public/copresence_constants.h"
 #include "media/base/audio_bus.h"
 
-namespace {
-
-// UrlSafe is defined as:
-// '/' represented by a '_' and '+' represented by a '-'
-// TODO(rkc): Move this processing to the whispernet wrapper.
-std::string FromUrlSafe(std::string token) {
-  base::ReplaceChars(token, "-", "+", &token);
-  base::ReplaceChars(token, "_", "/", &token);
-  return token;
-}
-
-const int kSampleExpiryTimeMs = 60 * 60 * 1000;  // 60 minutes.
-const int kMaxSamples = 10000;
-
-}  // namespace
-
 namespace copresence {
 
 // Public methods.
 
-AudioDirectiveHandler::AudioDirectiveHandler(
-    const AudioRecorder::DecodeSamplesCallback& decode_cb,
-    const AudioDirectiveHandler::EncodeTokenCallback& encode_cb)
-    : player_audible_(NULL),
-      player_inaudible_(NULL),
-      recorder_(NULL),
-      decode_cb_(decode_cb),
-      encode_cb_(encode_cb),
-      samples_cache_audible_(
-          base::TimeDelta::FromMilliseconds(kSampleExpiryTimeMs),
-          kMaxSamples),
-      samples_cache_inaudible_(
-          base::TimeDelta::FromMilliseconds(kSampleExpiryTimeMs),
-          kMaxSamples) {
+AudioDirectiveHandler::AudioDirectiveHandler()
+    : audio_manager_(make_scoped_ptr<AudioManager>(NULL)) {

[Daniel Erat, 2014/10/17 22:25:59]

don't think you need this; scoped_ptr's default c'tor initializes to NULL

[rkc, 2014/10/18 00:21:54]

Done.

 }
 
 AudioDirectiveHandler::~AudioDirectiveHandler() {
-  if (player_audible_)
-    player_audible_->Finalize();
-  if (player_inaudible_)
-    player_inaudible_->Finalize();
-  if (recorder_)
-    recorder_->Finalize();
 }
 
-void AudioDirectiveHandler::Initialize() {
-  player_audible_ = new AudioPlayer();
-  player_audible_->Initialize();
-
-  player_inaudible_ = new AudioPlayer();
-  player_inaudible_->Initialize();
-
-  recorder_ = new AudioRecorder(decode_cb_);
-  recorder_->Initialize();
+void AudioDirectiveHandler::Initialize(
+    const AudioManager::DecodeSamplesCallback& decode_cb,
+    const AudioManager::EncodeTokenCallback& encode_cb) {
+  audio_manager_ = make_scoped_ptr(new AudioManager());
+  audio_manager_->Initialize(decode_cb, encode_cb);
 }
 
 void AudioDirectiveHandler::AddInstruction(const TokenInstruction& instruction,
@@ -77,144 +37,124 @@ void AudioDirectiveHandler::AddInstruction(const TokenInstruction& instruction,
     case TRANSMIT:
       DVLOG(2) << "Audio Transmit Directive received. Token: "
                << instruction.token_id()
+               << " with medium= " << instruction.medium()
                << " with TTL=" << ttl.InMilliseconds();
       switch (instruction.medium()) {
         case AUDIO_ULTRASOUND_PASSBAND:
-          transmits_list_inaudible_.AddDirective(op_id, ttl);
-          PlayToken(instruction.token_id(), false);
+          transmits_list_[INAUDIBLE].AddDirective(op_id, ttl);
+          audio_manager_->SetToken(INAUDIBLE, instruction.token_id());
           break;
         case AUDIO_AUDIBLE_DTMF:
-          transmits_list_audible_.AddDirective(op_id, ttl);
-          PlayToken(instruction.token_id(), true);
+          transmits_list_[AUDIBLE].AddDirective(op_id, ttl);
+          audio_manager_->SetToken(AUDIBLE, instruction.token_id());
           break;
         default:
           NOTREACHED();
       }
       break;
     case RECEIVE:
-      DVLOG(2) << "Audio Receive Directive received. TTL="
-               << ttl.InMilliseconds();
-      receives_list_.AddDirective(op_id, ttl);
-      ProcessNextReceive();
+      DVLOG(2) << "Audio Receive Directive received."
+               << " with medium= " << instruction.medium()
+               << " with TTL=" << ttl.InMilliseconds();
+      switch (instruction.medium()) {
+        case AUDIO_ULTRASOUND_PASSBAND:
+          receives_list_[INAUDIBLE].AddDirective(op_id, ttl);
+          break;
+        case AUDIO_AUDIBLE_DTMF:
+          receives_list_[AUDIBLE].AddDirective(op_id, ttl);
+          break;
+        default:
+          NOTREACHED();
+      }
       break;
     case UNKNOWN_TOKEN_INSTRUCTION_TYPE:
     default:
-      LOG(WARNING) << "Unknown Audio Transmit Directive received.";
+      LOG(WARNING) << "Unknown Audio Transmit Directive received. type = "
+                   << instruction.token_instruction_type();
   }
+  ProcessNextInstruction();
 }
 
 void AudioDirectiveHandler::RemoveInstructions(const std::string& op_id) {
-  transmits_list_audible_.RemoveDirective(op_id);
-  transmits_list_inaudible_.RemoveDirective(op_id);
-  receives_list_.RemoveDirective(op_id);
+  transmits_list_[AUDIBLE].RemoveDirective(op_id);
+  transmits_list_[INAUDIBLE].RemoveDirective(op_id);
+  receives_list_[AUDIBLE].RemoveDirective(op_id);
+  receives_list_[INAUDIBLE].RemoveDirective(op_id);
 
-  ProcessNextTransmit();
-  ProcessNextReceive();
+  ProcessNextInstruction();
 }
 
 // Private methods.
 
-void AudioDirectiveHandler::ProcessNextTransmit() {
-  // If we have an active directive for audible or inaudible audio, ensure that
-  // we are playing our respective token; if we do not have a directive, then
-  // make sure we aren't playing. This is duplicate code, but for just two
-  // elements, it has hard to make a case for processing a loop instead.
-
-  scoped_ptr<AudioDirective> audible_transmit(
-      transmits_list_audible_.GetActiveDirective());
-  if (audible_transmit && !player_audible_->IsPlaying() &&
-      samples_cache_audible_.HasKey(current_token_audible_)) {
-    DVLOG(3) << "Playing audible for op_id: " << audible_transmit->op_id;
-    player_audible_->Play(
-        samples_cache_audible_.GetValue(current_token_audible_));
-    stop_audible_playback_timer_.Start(
-        FROM_HERE,
-        audible_transmit->end_time - base::Time::Now(),
-        this,
-        &AudioDirectiveHandler::ProcessNextTransmit);
-  } else if (!audible_transmit && player_audible_->IsPlaying()) {
-    DVLOG(3) << "Stopping audible playback.";
-    current_token_audible_.clear();
-    stop_audible_playback_timer_.Stop();
-    player_audible_->Stop();
-  }
+void AudioDirectiveHandler::ProcessNextInstruction() {
+  if (audio_event_timer_.IsRunning())
+    audio_event_timer_.Stop();
 
-  scoped_ptr<AudioDirective> inaudible_transmit(
-      transmits_list_inaudible_.GetActiveDirective());
-  if (inaudible_transmit && !player_inaudible_->IsPlaying() &&
-      samples_cache_inaudible_.HasKey(current_token_inaudible_)) {
-    DVLOG(3) << "Playing inaudible for op_id: " << inaudible_transmit->op_id;
-    player_inaudible_->Play(
-        samples_cache_inaudible_.GetValue(current_token_inaudible_));
-    stop_inaudible_playback_timer_.Start(
-        FROM_HERE,
-        inaudible_transmit->end_time - base::Time::Now(),
-        this,
-        &AudioDirectiveHandler::ProcessNextTransmit);
-  } else if (!inaudible_transmit && player_inaudible_->IsPlaying()) {
-    DVLOG(3) << "Stopping inaudible playback.";
-    current_token_inaudible_.clear();
-    stop_inaudible_playback_timer_.Stop();
-    player_inaudible_->Stop();
+  // Change audio_manager_ state for audible transmits.
+  if (transmits_list_[AUDIBLE].GetActiveDirective()) {

[Daniel Erat, 2014/10/17 22:25:59]

nit: omit curly brackets here and below

[rkc, 2014/10/18 00:21:54]

Done.

+    audio_manager_->StartPlaying(AUDIBLE);
+  } else {
+    audio_manager_->StopPlaying(AUDIBLE);
   }
-}
 
-void AudioDirectiveHandler::ProcessNextReceive() {
-  scoped_ptr<AudioDirective> receive(receives_list_.GetActiveDirective());
-
-  if (receive && !recorder_->IsRecording()) {
-    DVLOG(3) << "Recording for op_id: " << receive->op_id;
-    recorder_->Record();
-    stop_recording_timer_.Start(FROM_HERE,
-                                receive->end_time - base::Time::Now(),
-                                this,
-                                &AudioDirectiveHandler::ProcessNextReceive);
-  } else if (!receive && recorder_->IsRecording()) {
-    DVLOG(3) << "Stopping Recording";
-    stop_recording_timer_.Stop();
-    recorder_->Stop();
+  // Change audio_manager_ state for inaudible transmits.
+  if (transmits_list_[INAUDIBLE].GetActiveDirective()) {
+    audio_manager_->StartPlaying(INAUDIBLE);
+  } else {
+    audio_manager_->StopPlaying(INAUDIBLE);
   }
-}
 
-void AudioDirectiveHandler::PlayToken(const std::string token, bool audible) {
-  std::string valid_token = FromUrlSafe(token);
+  // Change audio_manager_ state for audible receives.
+  if (receives_list_[AUDIBLE].GetActiveDirective()) {
+    audio_manager_->StartRecording(AUDIBLE);
+  } else {
+    audio_manager_->StopRecording(AUDIBLE);
+  }
 
-  // If the token has been encoded already, use the cached samples.
-  if (audible && samples_cache_audible_.HasKey(valid_token)) {
-    current_token_audible_ = token;
-    ProcessNextTransmit();
-  } else if (!audible && samples_cache_inaudible_.HasKey(valid_token)) {
-    current_token_inaudible_ = token;
-    ProcessNextTransmit();
+  // Change audio_manager_ state for inaudible receives.
+  if (receives_list_[INAUDIBLE].GetActiveDirective()) {
+    audio_manager_->StartRecording(INAUDIBLE);
   } else {
-    // Otherwise, encode the token and then play it.
-    encode_cb_.Run(valid_token,
-                   audible,
-                   base::Bind(&AudioDirectiveHandler::PlayEncodedToken,
-                              base::Unretained(this)));
+    audio_manager_->StopRecording(INAUDIBLE);
+  }
+
+  base::TimeDelta next_event_time = NextInstructionExpiry();
+  if (next_event_time > base::TimeDelta::FromMilliseconds(0)) {
+    audio_event_timer_.Start(FROM_HERE,
+                             next_event_time,
+                             this,
+                             &AudioDirectiveHandler::ProcessNextInstruction);
   }
 }
 
-void AudioDirectiveHandler::PlayEncodedToken(
-    const std::string& token,
-    bool audible,
-    const scoped_refptr<media::AudioBusRefCounted>& samples) {
-  DVLOG(3) << "Token " << token << "[audible:" << audible << "] encoded.";
-  if (audible) {
-    samples_cache_audible_.Add(token, samples);
-    current_token_audible_ = token;
-    // Force process transmits to pick up the new token.
-    if (player_audible_->IsPlaying())
-      player_audible_->Stop();
-  } else {
-    samples_cache_inaudible_.Add(token, samples);
-    current_token_inaudible_ = token;
-    // Force process transmits to pick up the new token.
-    if (player_inaudible_->IsPlaying())
-      player_inaudible_->Stop();
+base::TimeDelta AudioDirectiveHandler::NextInstructionExpiry() {
+  // We cache now, since, calling Now() twice doesn't gaurantee that the time

[Daniel Erat, 2014/10/17 22:25:59]

nit: guarantee

[rkc, 2014/10/18 00:21:54]

Done.

+  // will increase (it may even decrease). This ensures that if we have no
+  // instructions, we return back a 0 time.
+  base::Time now = base::Time::Now();
+
+  base::Time closest_event_time = now;
+  if (transmits_list_[AUDIBLE].GetActiveDirective()) {
+    closest_event_time =
+        transmits_list_[AUDIBLE].GetActiveDirective()->end_time;
+  }
+  if (transmits_list_[INAUDIBLE].GetActiveDirective()) {

[Daniel Erat, 2014/10/17 22:25:59]

how about moving this into a helper function so it doesn't need to be duplicated
over and over?

[rkc, 2014/10/18 00:21:54]

Done.

+    closest_event_time =
+        std::min(transmits_list_[INAUDIBLE].GetActiveDirective()->end_time,
+                 closest_event_time);
+  }
+  if (receives_list_[AUDIBLE].GetActiveDirective()) {
+    closest_event_time =
+        std::min(receives_list_[AUDIBLE].GetActiveDirective()->end_time,
+                 closest_event_time);
+  }
+  if (receives_list_[INAUDIBLE].GetActiveDirective()) {
+    closest_event_time =
+        std::min(receives_list_[INAUDIBLE].GetActiveDirective()->end_time,
+                 closest_event_time);
   }
 
-  ProcessNextTransmit();
+  return closest_event_time - now;
 }
 
 }  // namespace copresence