Android implementation of WebAudio audio file decoder

media/base/android/webaudio_media_codec_bridge.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 "media/base/android/webaudio_media_codec_bridge.h"
+
+#include <unistd.h>
+#include <vector>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
+#include "base/android/jni_string.h"
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "jni/WebAudioMediaCodecBridge_jni.h"
+
+
+using base::android::AttachCurrentThread;
+
+namespace media {
+
+void WebAudioMediaCodecBridge::RunWebAudioMediaCodec(
+    base::SharedMemoryHandle encoded_audio_handle,
+    base::FileDescriptor pcm_output) {
+  DVLOG(0) << "RunWebAudioMediaCodec";
+
+  WebAudioMediaCodecBridge bridge(encoded_audio_handle, pcm_output);
+  bool result = bridge.DecodeInMemoryAudioFile();
+  DVLOG(0) << "RunWebAudioMediaCodec returned " << result;
+}
+
+WebAudioMediaCodecBridge::WebAudioMediaCodecBridge(
+    base::SharedMemoryHandle encoded_audio_handle,
+    base::FileDescriptor pcm_output)
+    : encoded_audio_handle_(encoded_audio_handle.fd),
+      pcm_output_(pcm_output.fd) {
+  DVLOG(0) << "WebAudioMediaCodecBridge start **********************"
+           << "input fd = " << encoded_audio_handle_
+           << " output fd = " << pcm_output.fd;
+}
+
+WebAudioMediaCodecBridge::~WebAudioMediaCodecBridge() {
+  int result = close(pcm_output_);
+  if (result)
+    DVLOG(0) << "Couldn't close output fd " << pcm_output_;

[palmer, 2013/04/10 00:57:53]

In both of these DVLOG messages, maybe it makes sense to use strerror(errno) to
describe the error? Up to you.

+
+  result = close(encoded_audio_handle_);
+  if (result)
+    DVLOG(0) << "Couldn't close shared mem fd " << encoded_audio_handle_;
+}
+
+bool WebAudioMediaCodecBridge::DecodeInMemoryAudioFile() {
+  // Process the encoded data that is in shared memory given by the

[palmer, 2013/04/10 00:57:53]

NIT: You could be more concise here. "Process the data from
|encoded_data_handle_|." We already know it's shared memory due to its type.

+  // file descriptor |encoded_data_handle_|.
+
+  JNIEnv* env = AttachCurrentThread();
+  CHECK(env);
+  jboolean decoded = Java_WebAudioMediaCodecBridge_decodeAudioFile(
+      env,
+      base::android::GetApplicationContext(),
+      reinterpret_cast<intptr_t>(this),
+      encoded_audio_handle_);
+
+  DVLOG(0) << "decoded = " << static_cast<bool>(decoded);
+  return decoded;
+}
+
+void WebAudioMediaCodecBridge::InitializeDestination(
+    JNIEnv* env,
+    jobject /*java object*/,
+    jint channel_count,
+    jint sample_rate,
+    jlong duration_microsec,
+    jboolean is_vorbis) {
+  // Send information about this audio file: number of channels,
+  // sample rate (Hz), number of frames, a flag indicating whether
+  // this file is an audio/vorbis file.  Information is sent as a set of
+  // 4 longs. This must be coordinated with DecodeAudioFileData!
+
+  long info[4] = {channel_count,
+                  sample_rate,
+                  // The number of frames is the duration of the file
+                  // (in microseconds) times the sample rate.
+                  0.5 + (duration_microsec * 0.000001 * sample_rate),
+                  is_vorbis ? 1 : 0};
+
+  DVLOG(0) << "InitializeDestination:"
+           << "  channel count = " << channel_count
+           << "  rate          = " << sample_rate
+           << "  duration      = " << duration_microsec
+           << " microsec vorbis= "

[palmer, 2013/04/10 00:57:53]

Did you mean just "vorbis" here?

[Raymond Toy (Google), 2013/04/10 19:23:36]

I wanted the duration to include the units.  I'll split this to make it clearer.

+           << (is_vorbis ? "yes" : "no");
+
+  write(pcm_output_, info, sizeof(info));
+}
+
+void WebAudioMediaCodecBridge::OnChunkDecoded(
+    JNIEnv* env,
+    jobject /*java object*/,
+    jobject buf,
+    jint buf_size) {
+  int16_t* decoded_buffer =
+      static_cast<int16_t*>(env->GetDirectBufferAddress(buf));
+  DCHECK((buf_size % sizeof(decoded_buffer[0])) == 0);
+
+  int16_t* buffer = decoded_buffer;

[palmer, 2013/04/10 00:57:53]

I still don't think you need both |buffer| and |decoded_buffer|, but it's up to
you.

+
+  // Write out the data to the pipe in small chunks if necessary.
+  while (buf_size > 0) {
+    int bytes_to_write = (buf_size >= PIPE_BUF) ? PIPE_BUF : buf_size;
+    ssize_t bytes_written = write(pcm_output_, buffer, bytes_to_write);
+    if (bytes_written == -1)
+      break;
+    if (bytes_written != bytes_to_write) {

[palmer, 2013/04/10 00:57:53]

This isn't really an exceptional condition worthy of VLOG; it's part of the
standard API for write(2). But, up to you.

+      VLOG(0) << "Only wrote " << bytes_written
+              << " bytes but expected " << bytes_to_write;
+    }
+    buf_size -= bytes_written;
+    buffer += bytes_written / sizeof(decoded_buffer[0]);

[palmer, 2013/04/10 00:57:53]

It's a bit confusing that we write in terms of bytes, but increment the buffer
in terms of int16_ts.

What if someday the samples are 24-bit? Could that ever happen?

Can we use int8_t* instead of int16_t* for decoded_buffer? Would make the code
arguably easier to verify correct, and would enable us to support 24-bit samples
without having to do any extra work.

Or, maybe I am wrong. :)

+  }
+}
+
+bool WebAudioMediaCodecBridge::RegisterWebAudioMediaCodecBridge(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+} // namespace