Support (E)AC3 passthrough

media/base/audio_buffer_queue.cc

 // Copyright 2013 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/audio_buffer_queue.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "media/base/audio_bus.h"
@@ skipping 17 matching lines @@
   current_buffer_ = buffers_.begin();
 
   // Update the |frames_| counter since we have added frames.
   frames_ += buffer_in->frame_count();
   CHECK_GT(frames_, 0);  // make sure it doesn't overflow.
 }
 
 int AudioBufferQueue::ReadFrames(int frames,
                                  int dest_frame_offset,
                                  AudioBus* dest) {
-  DCHECK_GE(dest->frames(), frames + dest_frame_offset);
+  if (!dest->is_bitstream_format())

[DaleCurtis, 2017/06/15 21:46:32]

Add corresponding dcheck for bitstream formats.

[AndyWu, 2017/08/02 01:43:40]

Done.

+    DCHECK_GE(dest->frames(), frames + dest_frame_offset);
   return InternalRead(frames, true, 0, dest_frame_offset, dest);
 }
 
 int AudioBufferQueue::PeekFrames(int frames,
                                  int source_frame_offset,
                                  int dest_frame_offset,
                                  AudioBus* dest) {
   DCHECK_GE(dest->frames(), frames);
   return InternalRead(

[DaleCurtis, 2017/06/15 21:46:32]

DCHECK that this is never called with a bitstream format?

[AndyWu, 2017/08/02 01:43:40]

Why do we need this restriction?

       frames, false, source_frame_offset, dest_frame_offset, dest);
 }
 
 void AudioBufferQueue::SeekFrames(int frames) {
   // Perform seek only if we have enough bytes in the queue.
   CHECK_LE(frames, frames_);
   int taken = InternalRead(frames, true, 0, 0, NULL);
   DCHECK_EQ(taken, frames);
 }
 
 int AudioBufferQueue::InternalRead(int frames,
                                    bool advance_position,
                                    int source_frame_offset,

[DaleCurtis, 2017/06/15 21:46:32]

Why do you need any changes here? If calling parameters are correct up and down
the chain the existing code should work fine, no?

[AndyWu, 2017/08/02 01:43:40]

For compressed bitstream formats, a partial compressed audio frame is less
useful, since it can't generate any PCM frame out of it. Also, we want to keep
the granularity as fine as possible so that discarding a small chunk of PCM
frames is still possible. Thus, we only transfer a complete AudioBuffer at a
time.

[DaleCurtis, 2017/08/03 01:38:10]

I guess I meant why not just DCHECK() that you have the right parameters when a
bitstream format is used.

[AndyWu, 2017/08/03 17:11:17]

Unfortunately, we don't know the right parameter during initialization stage
[1], so I use a reasonable large number, kMaxFramesPerCompressedAudioBuffer, to
only ensure the capacity of |dest|. Thus, the frame count of |dest| won't be the
same as the frame count of a AudioBuffer.

[1]
https://chromium-review.googlesource.com/c/596720/2/media/renderers/audio_ren...

                                    int dest_frame_offset,
                                    AudioBus* dest) {
+  if (!buffers_.size())

[DaleCurtis, 2017/06/15 21:46:32]

if buffers_.empty()

[AndyWu, 2017/08/02 01:43:40]

Done.

+    return 0;
+
+  if ((*buffers_.begin())->IsBitstreamFormat()) {

[chcunningham, 2017/06/14 20:03:08]

This block needs a comment to explain why |frames| is totally ignored in this
case (you're just reading the whole of the front buffer)

[DaleCurtis, 2017/06/15 21:46:32]

Just buffers_.front()->IsBitstreamFormat()

[AndyWu, 2017/08/02 01:43:40]

Done.

[AndyWu, 2017/08/02 01:43:40]

Done.

+    DCHECK(!dest_frame_offset);
+    DCHECK(!source_frame_offset);
+
+    scoped_refptr<AudioBuffer> buffer = buffers_.front();

[chcunningham, 2017/06/14 20:03:08]

Should you use current_buffer_ here to be consistent with original code below?

[AndyWu, 2017/08/02 01:43:40]

Done.

+    int taken = buffer->frame_count();
+
+    // if |dest| is NULL, there's no need to copy.
+    if (dest)
+      buffer->ReadFrames(buffer->frame_count(), 0, dest->data_size(), dest);

[chcunningham, 2017/06/14 20:03:08]

Do you mean to pass dest->data_size() here? Shouldn't this be
|dest_frame_offset|? 

[AndyWu, 2017/08/02 01:43:40]

You are right!
I was trying to append the data to the end of existing data, so I passed
data_size() instead of |dest_frame_offset|. It's tricky and bad.

+
+    if (advance_position) {
+      // Update the appropriate values since |taken| frames have been copied
+      // out.
+      frames_ -= taken;
+      DCHECK_GE(frames_, 0);
+
+      // Remove any buffers before the current buffer as there is no going
+      // backwards.
+      buffers_.pop_front();
+      current_buffer_ = buffers_.begin();
+    }
+
+    return taken;
+  }
+
   // Counts how many frames are actually read from the buffer queue.
   int taken = 0;
   BufferQueue::iterator current_buffer = current_buffer_;
   int current_buffer_offset = current_buffer_offset_;
 
   int frames_to_skip = source_frame_offset;
   while (taken < frames) {
     // |current_buffer| is valid since the first time this buffer is appended
     // with data. Make sure there is data to be processed.
     if (current_buffer == buffers_.end())
@@ skipping 54 matching lines @@
     // backwards.
     buffers_.erase(buffers_.begin(), current_buffer);
     current_buffer_ = buffers_.begin();
     current_buffer_offset_ = current_buffer_offset;
   }
 
   return taken;
 }
 
 }  // namespace media