Implement renderer AudioDevice API for low-latency audio output...

chrome/renderer/audio_device.h

+// Copyright (c) 2010 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.
+
+#ifndef CHROME_RENDERER_AUDIO_DEVICE_H_
+#define CHROME_RENDERER_AUDIO_DEVICE_H_
+#pragma once
+
+#include <vector>

[scherkus (not reviewing), 2011/01/10 23:42:32]

nit: blank line between system + non-system headers

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+#include "base/basictypes.h"
+#include "base/scoped_ptr.h"
+#include "base/shared_memory.h"
+#include "base/threading/simple_thread.h"
+#include "chrome/common/render_messages.h"
+#include "chrome/renderer/audio_message_filter.h"
+
+// Each instance of AudioDevice corresponds to one host stream.

[scherkus (not reviewing), 2011/01/10 23:42:32]

are there any threading restrictions as to how AudioDevices are used?

should AudioDevice only be accessed on the main render thread or can any thread
create/delete an AudioDevice, call Stop/Start, etc...

[Chris Rogers, 2011/01/11 23:01:02]

I've added a comment that the class is *not* thread-safe.

On 2011/01/10 23:42:32, scherkus wrote:

+class AudioDevice : public AudioMessageFilter::Delegate,
+                    public base::DelegateSimpleThread::Delegate {
+ public:
+  class RenderCallback {

[scherkus (not reviewing), 2011/01/10 23:42:32]

what about typedeffing a callback type instead of introducing a type?

typedef Callback2<const std::vector<float*>&, size_t>::Type RenderCallback;

It's mostly a preference/style thing.

--UPDATE--

I think you're reflecting the name of the callback in WebKit -- in which case I
think this is OK (unless that's not what you're doing).

[Chris Rogers, 2011/01/11 23:01:02]

Yes, it's pretty much mirroring what we're doing in the WebKit API.

On 2011/01/10 23:42:32, scherkus wrote:

+   public:
+    virtual void render(const std::vector<float*>& audio_data,

[scherkus (not reviewing), 2011/01/10 23:42:32]

considering this isn't a non-trivial function it should be capitalized: Render

--UPDATE--

I see that this is trying to duplicate the WebKit RenderCallback interface,
however this is straight up Chrome code so the capitalized Render() still sticks

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+                        size_t number_of_frames) = 0;
+   protected:
+    virtual ~RenderCallback() { }

[scherkus (not reviewing), 2011/01/10 23:42:32]

no inlined ctors/dtors in header files

http://dev.chromium.org/developers/coding-style#TOC-Inline-functions

[Chris Rogers, 2011/01/11 23:01:02]

I agree with the style guideline in general, but I'm not sure that it's meant to
apply to abstract base classes like this.  The media directory has a few
abstract base classes where the virtual destructor is inline:

class AudioManager;
class AudioOutputStream;
class FFmpegURLProtocol;


On 2011/01/10 23:42:32, scherkus wrote:

+  };
+
+  AudioDevice(size_t buffer_size,
+              int channels,
+              double sample_rate,
+              RenderCallback* callback);
+  virtual ~AudioDevice();
+
+  // Returns |true| on success.
+  bool Start();
+  bool Stop();
+
+ private:
+  // AudioMessageFilter::Delegate implementation

[scherkus (not reviewing), 2011/01/10 23:42:32]

nit: period at end

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+  virtual void OnRequestPacket(AudioBuffersState buffers_state);
+  virtual void OnStateChanged(const ViewMsg_AudioStreamState_Params& state);
+  virtual void OnCreated(base::SharedMemoryHandle handle, uint32 length);
+  virtual void OnLowLatencyCreated(base::SharedMemoryHandle handle,
+                                   base::SyncSocket::Handle socket_handle,
+                                   uint32 length);
+  virtual void OnVolume(double volume);
+  virtual void OnDestroy();
+  // End of AudioMessageFilter::Delegate implementation

[scherkus (not reviewing), 2011/01/10 23:42:32]

nit: based on recent chromium-dev style conversations there's no need for this
comment

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+
+  // DelegateSimpleThread::Delegate implementation

[scherkus (not reviewing), 2011/01/10 23:42:32]

nit: period at end

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+  virtual void Run();
+  // End of DelegateSimpleThread::Delegate implementation

[scherkus (not reviewing), 2011/01/10 23:42:32]

ditto

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+
+  // Format
+  size_t buffer_size_;
+  int channels_;
+  double sample_rate_;
+
+  // Calls the client's callback for rendering audio.
+  void FireRenderCallback();
+  RenderCallback* callback_;
+
+  // The client callback renders audio into here.
+  std::vector<float*> audio_data_;
+
+  // Callbacks for rendering audio occur on this thread.
+  scoped_ptr<base::DelegateSimpleThread> audio_thread_;
+
+  // IPC message stuff.
+  static scoped_refptr<AudioMessageFilter> filter_;
+  int32 stream_id_;
+  scoped_ptr<base::SharedMemory> shared_memory_;
+  scoped_ptr<base::SyncSocket> socket_;
+
+  base::SharedMemory* shared_memory() { return shared_memory_.get(); }

[scherkus (not reviewing), 2011/01/10 23:42:32]

nit: class order style has helper methods go before data memebers

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+  base::SyncSocket* socket() { return socket_.get(); }
+  void* shared_memory_data() { return shared_memory()->memory(); }
+};

[scherkus (not reviewing), 2011/01/10 23:42:32]

DISALLOW_COPY_AND_ASSIGN

[Chris Rogers, 2011/01/11 23:01:02]

Done.

+
+#endif  // CHROME_RENDERER_AUDIO_DEVICE_H_