device/bluetooth: Add I/O watcher for audio data retrieval triggered by state change.

device/bluetooth/bluetooth_audio_sink_chromeos.cc

 // Copyright 2015 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 "device/bluetooth/bluetooth_audio_sink_chromeos.h"
 
+#include <unistd.h>
+
 #include <algorithm>
 #include <sstream>
 #include <string>
 #include <vector>
 
 #include "base/debug/stack_trace.h"
+#include "base/files/file_util.h"
 #include "base/logging.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "dbus/message.h"
 #include "device/bluetooth/bluetooth_adapter_chromeos.h"
 
 using dbus::ObjectPath;
 using device::BluetoothAudioSink;
 
 namespace {
 
 // TODO(mcchou): Add the constant to dbus/service_constants.h.
 const char kBluetoothAudioSinkServicePath[] = "/org/chromium/AudioSink";
 
+const int kInvalidFd = -1;
+const uint16_t kInvalidReadMtu = 0;
+const uint16_t kInvalidWriteMtu = 0;
+
 ObjectPath GenerateEndpointPath() {
   static unsigned int sequence_number = 0;
   ++sequence_number;
   std::stringstream path;
   path << kBluetoothAudioSinkServicePath << "/endpoint" << sequence_number;
   return ObjectPath(path.str());
 }
 
 std::string StateToString(const BluetoothAudioSink::State& state) {
   switch (state) {
@@ skipping 35 matching lines @@
 }
 
 }  // namespace
 
 namespace chromeos {
 
 BluetoothAudioSinkChromeOS::BluetoothAudioSinkChromeOS(
     scoped_refptr<device::BluetoothAdapter> adapter)
     : state_(BluetoothAudioSink::STATE_INVALID),
       volume_(BluetoothAudioSink::kInvalidVolume),
-      read_mtu_(nullptr),
-      write_mtu_(nullptr),
+      read_mtu_(kInvalidReadMtu),
+      write_mtu_(kInvalidWriteMtu),
+      read_failed_logged_(false),
       adapter_(adapter),
       weak_ptr_factory_(this) {
   VLOG(1) << "BluetoothAudioSinkChromeOS created";
 
   DCHECK(adapter_.get());
   DCHECK(adapter_->IsPresent());
+  DCHECK(DBusThreadManager::IsInitialized());
 
   adapter_->AddObserver(this);
 
-  chromeos::BluetoothMediaClient* media =
+  BluetoothMediaClient* media =
       DBusThreadManager::Get()->GetBluetoothMediaClient();
   DCHECK(media);
   media->AddObserver(this);
 
-  chromeos::BluetoothMediaTransportClient *transport =
-      chromeos::DBusThreadManager::Get()->GetBluetoothMediaTransportClient();
+  BluetoothMediaTransportClient* transport =
+      DBusThreadManager::Get()->GetBluetoothMediaTransportClient();
   DCHECK(transport);
   transport->AddObserver(this);
 
   StateChanged(device::BluetoothAudioSink::STATE_DISCONNECTED);
 }
 
 BluetoothAudioSinkChromeOS::~BluetoothAudioSinkChromeOS() {
   VLOG(1) << "BluetoothAudioSinkChromeOS destroyed";
 
   DCHECK(adapter_.get());
 
   if (state_ != BluetoothAudioSink::STATE_INVALID && media_endpoint_.get()) {
     Unregister(base::Bind(&base::DoNothing),
                base::Bind(&UnregisterErrorCallback));
   }
 
   adapter_->RemoveObserver(this);
 
-  chromeos::BluetoothMediaClient* media =
+  BluetoothMediaClient* media =
       DBusThreadManager::Get()->GetBluetoothMediaClient();
   DCHECK(media);
   media->RemoveObserver(this);
 
-  chromeos::BluetoothMediaTransportClient *transport =
-      chromeos::DBusThreadManager::Get()->GetBluetoothMediaTransportClient();
+  BluetoothMediaTransportClient* transport =
+      DBusThreadManager::Get()->GetBluetoothMediaTransportClient();
   DCHECK(transport);
   transport->RemoveObserver(this);
 }
 
 void BluetoothAudioSinkChromeOS::Unregister(
     const base::Closure& callback,
     const device::BluetoothAudioSink::ErrorCallback& error_callback) {
   VLOG(1) << "Unregister";
 
   if (!DBusThreadManager::IsInitialized())
     error_callback.Run(BluetoothAudioSink::ERROR_NOT_UNREGISTERED);
 
-  chromeos::BluetoothMediaClient* media =
+  BluetoothMediaClient* media =
       DBusThreadManager::Get()->GetBluetoothMediaClient();
   DCHECK(media);
 
   media->UnregisterEndpoint(
       media_path_,
       endpoint_path_,
       base::Bind(&BluetoothAudioSinkChromeOS::OnUnregisterSucceeded,
                  weak_ptr_factory_.GetWeakPtr(), callback),
       base::Bind(&BluetoothAudioSinkChromeOS::OnUnregisterFailed,
                  weak_ptr_factory_.GetWeakPtr(), error_callback));
@@ skipping 63 matching lines @@
 
 void BluetoothAudioSinkChromeOS::MediaTransportPropertyChanged(
     const ObjectPath& object_path,
     const std::string& property_name) {
   if (object_path != transport_path_)
     return;
 
   VLOG(1) << "MediaTransportPropertyChanged: " << property_name;
 
   // Retrieves the property set of the transport object with |object_path|.
-  chromeos::BluetoothMediaTransportClient::Properties* properties =
+  BluetoothMediaTransportClient::Properties* properties =
       DBusThreadManager::Get()
           ->GetBluetoothMediaTransportClient()
           ->GetProperties(object_path);
 
   // Dispatches a property changed event to the corresponding handler.
   if (property_name == properties->state.name()) {
     if (properties->state.value() ==
         BluetoothMediaTransportClient::kStateIdle) {
       StateChanged(BluetoothAudioSink::STATE_IDLE);
     } else if (properties->state.value() ==
@@ skipping 32 matching lines @@
     const std::vector<uint8_t>& capabilities,
     const SelectConfigurationCallback& callback) {
   VLOG(1) << "SelectConfiguration";
   callback.Run(options_.capabilities);
 }
 
 void BluetoothAudioSinkChromeOS::ClearConfiguration(
     const ObjectPath& transport_path) {
   if (transport_path != transport_path_)
     return;
+
   VLOG(1) << "ClearConfiguration";
   StateChanged(BluetoothAudioSink::STATE_DISCONNECTED);
 }
 
 void BluetoothAudioSinkChromeOS::Released() {
   VLOG(1) << "Released";
   StateChanged(BluetoothAudioSink::STATE_INVALID);
 }
 
 void BluetoothAudioSinkChromeOS::Register(
@@ skipping 19 matching lines @@
   // Creates endpoint properties with |options|.
   options_ = options;
   chromeos::BluetoothMediaClient::EndpointProperties endpoint_properties;
   endpoint_properties.uuid = BluetoothMediaClient::kBluetoothAudioSinkUUID;
   endpoint_properties.codec = options_.codec;
   endpoint_properties.capabilities = options_.capabilities;
 
   media_path_ = static_cast<BluetoothAdapterChromeOS*>(
       adapter_.get())->object_path();
 
-  chromeos::BluetoothMediaClient* media =
+  BluetoothMediaClient* media =
       DBusThreadManager::Get()->GetBluetoothMediaClient();
   DCHECK(media);
   media->RegisterEndpoint(
       media_path_,
       endpoint_path_,
       endpoint_properties,
       base::Bind(&BluetoothAudioSinkChromeOS::OnRegisterSucceeded,
                  weak_ptr_factory_.GetWeakPtr(), callback),
       base::Bind(&BluetoothAudioSinkChromeOS::OnRegisterFailed,
                  weak_ptr_factory_.GetWeakPtr(), error_callback));
 }
 
+void BluetoothAudioSinkChromeOS::OnFileCanReadWithoutBlocking(int fd) {
+  ReadFromFile();
+}
+
+void BluetoothAudioSinkChromeOS::OnFileCanWriteWithoutBlocking(int fd) {
+  // Do nothing for now.
+}
+
 BluetoothMediaEndpointServiceProvider*
     BluetoothAudioSinkChromeOS::GetEndpointServiceProvider() {
   return media_endpoint_.get();
 }
 
+void BluetoothAudioSinkChromeOS::AcquireFD() {
+  VLOG(1) << "AcquireFD - transport path: " << transport_path_.value();
+
+  read_failed_logged_ = false;

[armansito, 2015/03/12 03:42:55]

I would rename this variable to something like |read_has_failed_|, so that it's
a bit more generic and you could reuse it for things other than logging in the
future.

[Miao, 2015/03/12 22:33:32]

Done.

+
+  DBusThreadManager::Get()->GetBluetoothMediaTransportClient()->Acquire(
+      transport_path_,
+      base::Bind(&BluetoothAudioSinkChromeOS::OnAcquireSucceeded,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&BluetoothAudioSinkChromeOS::OnAcquireFailed,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void BluetoothAudioSinkChromeOS::WatchFD() {
+  DCHECK(file_.get() && file_->IsValid());
+
+  VLOG(1) << "WatchFD - file: " << file_->GetPlatformFile()
+          << ", file validity: " << file_->IsValid();
+
+  base::MessageLoopForIO::current()->WatchFileDescriptor(
+      file_->GetPlatformFile(), true, base::MessageLoopForIO::WATCH_READ,
+      &fd_read_watcher_, this);
+}
+
+void BluetoothAudioSinkChromeOS::StopWatchingFD() {
+  if (!file_.get()) {
+    VLOG(1) << "StopWatchingFD - skip";
+    return;
+  }
+
+  bool stopped = fd_read_watcher_.StopWatchingFileDescriptor();
+  VLOG(1) << "StopWatchingFD - watch stopped: " << stopped;
+  CHECK(stopped);
+
+  read_mtu_ = kInvalidReadMtu;
+  write_mtu_ = kInvalidWriteMtu;
+  file_.reset();

[armansito, 2015/03/12 03:42:55]

Add a comment here saying that this will close the fd.

[Miao, 2015/03/12 22:33:32]

Done.

+}
+
+void BluetoothAudioSinkChromeOS::ReadFromFile() {
+  DCHECK(file_.get() && file_->IsValid());
+  DCHECK(data_.get());
+
+  int size = file_->ReadAtCurrentPosNoBestEffort(data_.get(), read_mtu_);
+
+  if (size == -1) {
+    // To reduce the number of logs, log only once for multiple failures.
+    if (!read_failed_logged_) {
+      VLOG(1) << "ReadFromFile - failed";
+      read_failed_logged_ = true;
+    }
+    return;
+  }
+
+  VLOG(1) << "ReadFromFile - read " << size << " bytes";
+  FOR_EACH_OBSERVER(BluetoothAudioSink::Observer, observers_,
+                    BluetoothAudioSinkDataAvailable(this, data_.get(), size));
+}
+
 void BluetoothAudioSinkChromeOS::StateChanged(
     BluetoothAudioSink::State state) {
   if (state == state_)
     return;
 
-  VLOG(1) << "StateChnaged: " << StateToString(state);
+  VLOG(1) << "StateChanged - state: " << StateToString(state);
 
   switch (state) {
     case BluetoothAudioSink::STATE_INVALID:
       ResetMedia();
       ResetEndpoint();
     case BluetoothAudioSink::STATE_DISCONNECTED:
       ResetTransport();
       break;
     case BluetoothAudioSink::STATE_IDLE:
-      // TODO(mcchou): BUG=441581
-      // Triggered by MediaTransportPropertyChanged and SetConfiguration.
-      // Stop watching on file descriptor if there is one.
+      StopWatchingFD();
       break;
     case BluetoothAudioSink::STATE_PENDING:
-      // TODO(mcchou): BUG=441581
-      // Call BluetoothMediaTransportClient::Acquire() to get fd and mtus.
+      AcquireFD();
       break;
     case BluetoothAudioSink::STATE_ACTIVE:
-      // TODO(mcchou): BUG=441581
-      // Read from fd and call DataAvailable.
-      ReadFromFD();
+      WatchFD();
       break;
     default:
       break;
   }
 
   state_ = state;
   FOR_EACH_OBSERVER(BluetoothAudioSink::Observer, observers_,
                     BluetoothAudioSinkStateChanged(this, state_));
 }
 
@@ skipping 23 matching lines @@
     const std::string& error_message) {
   VLOG(1) << "OnRegisterFailed - error name: " << error_name
           << ", error message: " << error_message;
 
   ResetEndpoint();
   error_callback.Run(BluetoothAudioSink::ERROR_NOT_REGISTERED);
 }
 
 void BluetoothAudioSinkChromeOS::OnUnregisterSucceeded(
     const base::Closure& callback) {
-  VLOG(1) << "Unregisterd";
+  VLOG(1) << "Unregisterd - endpoint: " << endpoint_path_.value();

[armansito, 2015/03/12 03:42:55]

nit: s/Unregisterd/Unregistered/

[Miao, 2015/03/12 22:33:32]

Done.

 
   // Once the state becomes STATE_INVALID, media, media transport and media
   // endpoint will be reset.
   StateChanged(BluetoothAudioSink::STATE_INVALID);
   callback.Run();
 }
 
 void BluetoothAudioSinkChromeOS::OnUnregisterFailed(
     const device::BluetoothAudioSink::ErrorCallback& error_callback,
     const std::string& error_name,
     const std::string& error_message) {
   VLOG(1) << "OnUnregisterFailed - error name: " << error_name
           << ", error message: " << error_message;
 
   error_callback.Run(BluetoothAudioSink::ERROR_NOT_UNREGISTERED);
 }
 
-void BluetoothAudioSinkChromeOS::ReadFromFD() {
-  DCHECK_GE(fd_.value(), 0);
+void BluetoothAudioSinkChromeOS::OnAcquireSucceeded(
+    const int fd,
+    const uint16_t read_mtu,
+    const uint16_t write_mtu) {
+  DCHECK_GT(fd, kInvalidFd);
+  DCHECK_GT(read_mtu, kInvalidReadMtu);
+  DCHECK_GT(write_mtu, kInvalidWriteMtu);
 
-  // TODO(mcchou): BUG=441581
-  // Read from file descriptor using watcher and create a buffer to contain the
-  // data. Notify |Observers_| while there is audio data available.
+  // Avoids unnecessary memory reallocation if read MTU doesn't change.

[armansito, 2015/03/12 03:42:55]

Nice!

[Miao, 2015/03/12 22:33:32]

:)

+  if (read_mtu != read_mtu_) {
+    read_mtu_ = read_mtu;
+    data_.reset(new char[read_mtu_]);
+    VLOG(1) << "OnAcquireSucceeded - allocate " << read_mtu_
+            << " bytes of memory";
+  }
+
+  write_mtu_ = write_mtu;
+
+  // Avoids closing the same file descriptor caused by reassignment.
+  if (!file_.get() || file_->GetPlatformFile() != fd) {
+    file_.reset(new base::File(fd));
+    DCHECK(file_->IsValid());
+    VLOG(1) << "OnAcquireSucceeded - update file";
+  }
+
+  VLOG(1) << "OnAcquireSucceeded - file: " << file_->GetPlatformFile()
+          << ", read MTU: " << read_mtu_ << ", write MTU: " << write_mtu_;
+}
+
+void BluetoothAudioSinkChromeOS::OnAcquireFailed(
+    const std::string& error_name,
+    const std::string& error_message) {
+  VLOG(1) << "OnAcquireFailed - error name: " << error_name
+          << ", error message: " << error_message;
+}
+
+void BluetoothAudioSinkChromeOS::OnReleaseFDSucceeded() {
+  VLOG(1) << "OnReleaseFDSucceeded";
+}
+
+void BluetoothAudioSinkChromeOS::OnReleaseFDFailed(
+    const std::string& error_name,
+    const std::string& error_message) {
+  VLOG(1) << "OnReleaseFDFailed - error name: " << error_name
+          << ", error message: " << error_message;
 }
 
 void BluetoothAudioSinkChromeOS::ResetMedia() {
   VLOG(1) << "ResetMedia";
 
   media_path_ = dbus::ObjectPath("");
 }
 
 void BluetoothAudioSinkChromeOS::ResetTransport() {
-  VLOG(1) << "ResetTransport";
+  if (!transport_path_.IsValid()) {
+    VLOG(1) << "ResetTransport - skip";
+    return;
+  }
 
-  if (transport_path_.value() == "")
-    return;
+  VLOG(1) << "ResetTransport - clean-up";
+
+  VolumeChanged(BluetoothAudioSink::kInvalidVolume);
   transport_path_ = dbus::ObjectPath("");
-  VolumeChanged(BluetoothAudioSink::kInvalidVolume);
-  read_mtu_ = 0;
-  write_mtu_ = 0;
-  fd_.PutValue(-1);
+  read_mtu_ = kInvalidReadMtu;
+  write_mtu_ = kInvalidWriteMtu;
+  file_.reset();
 }
 
 void BluetoothAudioSinkChromeOS::ResetEndpoint() {
   VLOG(1) << "ResetEndpoint";
 
   endpoint_path_ = ObjectPath("");
   media_endpoint_ = nullptr;
 }
 
 }  // namespace chromeos