Move AudioDebugFileWriter from content/ to media/.

media/audio/audio_debug_file_writer.h

-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2017 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 MEDIA_AUDIO_AUDIO_FILE_WRITER_H_
-#define MEDIA_AUDIO_AUDIO_FILE_WRITER_H_
+#ifndef MEDIA_AUDIO_AUDIO_DEBUG_FILE_WRITER_H_
+#define MEDIA_AUDIO_AUDIO_DEBUG_FILE_WRITER_H_
+
+#include <stdint.h>
 
 #include <memory>
 
-namespace base {
-class FilePath;
-}
+#include "base/files/file.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/sequence_checker.h"
+#include "base/single_thread_task_runner.h"
+#include "media/base/audio_parameters.h"
+#include "media/base/media_export.h"
 
 namespace media {
 
 class AudioBus;
 
-// A writer interface used for writing audio data to file for debugging
-// purposes.
-class AudioFileWriter {
+// Writes audio data used for debugging purposes. All operations are
+// non-blocking.
+class MEDIA_EXPORT AudioDebugFileWriter {
  public:
-  virtual ~AudioFileWriter() {}
+  AudioDebugFileWriter(
+      const media::AudioParameters& params,

[o1ka, 2017/02/21 12:07:44]

nit: remove media prefix everywhere in media namespace

[Henrik Grunell, 2017/02/21 14:10:52]

Missed that. Done.

+      scoped_refptr<base::SingleThreadTaskRunner> file_task_runner);
+  ~AudioDebugFileWriter();
 
   // Must be called before calling Write() for the first time after creation or
   // Stop() call. Can be called on any sequence; Write() and Stop() must be
   // called on the same sequence as Start().
-  virtual void Start(const base::FilePath& file) = 0;
+  void Start(const base::FilePath& file);
 
   // Must be called to finish recording. Each call to Start() requires a call to
   // Stop(). Will be automatically called on destruction.
-  virtual void Stop() = 0;
+  void Stop();
 
   // Write |data| to file.
-  virtual void Write(std::unique_ptr<AudioBus> data) = 0;
+  void Write(std::unique_ptr<media::AudioBus> data);
 
   // Returns true if Write() call scheduled at this point will most likely write
   // data to the file, and false if it most likely will be a no-op. The result
   // may be ambigulous if Start() or Stop() is executed at the moment. Can be
   // called from any sequence.
-  virtual bool WillWrite() = 0;
+  bool WillWrite();
+
+ private:
+  class AudioFileWriter;
+  struct DeleteOnFileThread;

[o1ka, 2017/02/21 12:07:45]

Why do you need forward declarations if classes are declared just below?

[Henrik Grunell, 2017/02/21 14:10:52]

Because of the

using AudioFileWriterUniquePtr ...

statement, which needs to be before AudioFileWriter declaration, because of

AudioFileWriterUniquePtr Create().

AudioFileWriter declaration, in turn, needs to be before DeleteOnFileThread
declaration.

Anyway, I moved the AFW declaration and the deleter definition to the impl file.

+
+  using AudioFileWriterUniquePtr =
+      std::unique_ptr<AudioFileWriter, DeleteOnFileThread>;
+
+  // Manages the debug recording file and writes to it. Can be created on any
+  // thread. All the operations must be executed on |file_task_runner_|. Must be
+  // destroyed on |file_task_runner_|.
+  class AudioFileWriter {

[o1ka, 2017/02/21 12:07:44]

Why is it needed in the header?

[Henrik Grunell, 2017/02/21 14:10:52]

It was because of the deleter definition below (called into it). Both are moved
now.

+   public:
+    static AudioFileWriterUniquePtr Create(
+        const base::FilePath& file_name,
+        const media::AudioParameters& params,
+        scoped_refptr<base::SingleThreadTaskRunner> file_task_runner);
+
+    ~AudioFileWriter();
+
+    // Write data from |data| to file.
+    void Write(const media::AudioBus* data);
+
+    scoped_refptr<base::SingleThreadTaskRunner> file_task_runner() {
+      return file_task_runner_;
+    }
+
+   private:
+    AudioFileWriter(
+        const media::AudioParameters& params,
+        scoped_refptr<base::SingleThreadTaskRunner> file_task_runner);
+
+    // Write wave header to file. Called on the |file_task_runner_| twice: on
+    // construction
+    // of AudioFileWriter size of the wave data is unknown, so the header is
+    // written with zero sizes; then on destruction it is re-written with the
+    // actual size info accumulated throughout the object lifetime.
+    void WriteHeader();
+
+    void CreateRecordingFile(const base::FilePath& file_name);
+
+    // The file to write to.
+    base::File file_;
+
+    // Number of written samples.
+    uint64_t samples_;
+
+    // Input audio parameters required to build wave header.
+    const media::AudioParameters params_;
+
+    // Intermediate buffer to be written to file. Interleaved 16 bit audio data.
+    std::unique_ptr<int16_t[]> interleaved_data_;
+    int interleaved_data_size_;
+
+    // The task runner this class operates on.
+    scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;

[o1ka, 2017/02/21 12:07:45]

just |task_runner_|?

[Henrik Grunell, 2017/02/21 14:10:52]

Sounds good, done.

+  };
+
+  // AudioFileWriter deleter. Inspired by
+  // content::BrowserThread::DeleteOnFileThread.
+  struct DeleteOnFileThread {
+    void operator()(AudioFileWriter* ptr) const {
+      if (!ptr->file_task_runner()->DeleteSoon(FROM_HERE, ptr)) {

[o1ka, 2017/02/21 12:07:45]

Initialize struct with the task runner, instead of having getter?
In this case it can be called "DeleteOnThread" or "OnThreadDeleter".

[o1ka, 2017/02/21 12:07:45]

Move implementation to cc?

[Henrik Grunell, 2017/02/21 14:10:52]

I started writing this change but didn't have time to finish it, continuing
tomorrow.

[Henrik Grunell, 2017/02/21 14:10:52]

Ah, yes. Done. (The declaration needs to remain for

AudioFileWriterUniquePtr file_writer_;

below.)

[Henrik Grunell, 2017/02/22 10:47:42]

Now done.

+#if defined(UNIT_TEST)
+        // Only logged under unit testing because leaks at shutdown
+        // are acceptable under normal circumstances.
+        LOG(ERROR)
+            << "DeleteSoon failed for AudioDebugFileWriter::AudioFileWriter";
+#endif
+      }
+    }
+  };
+
+  AudioFileWriterUniquePtr file_writer_;
+  const media::AudioParameters params_;

[o1ka, 2017/02/21 12:07:45]

nit: media:: not needed

[Henrik Grunell, 2017/02/21 14:10:52]

Done.

+  base::SequenceChecker client_sequence_checker_;
+
+  // The task runner to do file IO operations on.

[o1ka, 2017/02/21 12:07:44]

just O operations, not I, right? :) we are only writing.

[Henrik Grunell, 2017/02/21 14:10:52]

:) Sure, but I'd claim IO isn't a literal term in that sense. No point
bike-shedding - changed.

+  scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
 };
 
-}  // namespace media
+}  // namspace media
 
-#endif  // MEDIA_AUDIO_AUDIO_FILE_WRITER_H_
+#endif  // MEDIA_AUDIO_AUDIO_DEBUG_FILE_WRITER_H_