Fix incorrect opus seek preroll and flaky pre-skip removal.

media/filters/opus_audio_decoder.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/filters/opus_audio_decoder.h"
 
 #include <cmath>
 
 #include "base/single_thread_task_runner.h"
 #include "base/sys_byteorder.h"
@@ skipping 227 matching lines @@
   if (extra_data->num_streams + extra_data->num_coupled != extra_data->channels)
     DVLOG(1) << "Inconsistent channel mapping.";
 
   for (int i = 0; i < extra_data->channels; ++i)
     extra_data->stream_map[i] = *(data + kOpusExtraDataStreamMapOffset + i);
   return true;
 }
 
 OpusAudioDecoder::OpusAudioDecoder(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
-    : task_runner_(task_runner),
-      opus_decoder_(NULL),
-      start_input_timestamp_(kNoTimestamp()) {}
+    : task_runner_(task_runner), opus_decoder_(nullptr) {}
 
 std::string OpusAudioDecoder::GetDisplayName() const {
   return "OpusAudioDecoder";
 }
 
 void OpusAudioDecoder::Initialize(const AudioDecoderConfig& config,
                                   const InitCB& init_cb,
                                   const OutputCB& output_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   InitCB bound_init_cb = BindToCurrentLoop(init_cb);
@@ skipping 25 matching lines @@
   task_runner_->PostTask(FROM_HERE, closure);
 }
 
 OpusAudioDecoder::~OpusAudioDecoder() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   if (!opus_decoder_)
     return;
 
   opus_multistream_decoder_ctl(opus_decoder_, OPUS_RESET_STATE);
-  ResetTimestampState();
   CloseDecoder();
 }
 
 void OpusAudioDecoder::DecodeBuffer(
     const scoped_refptr<DecoderBuffer>& input,
     const DecodeCB& decode_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!decode_cb.is_null());
   DCHECK(input.get());
 
   // Libopus does not buffer output. Decoding is complete when an end of stream
   // input buffer is received.
   if (input->end_of_stream()) {
     decode_cb.Run(kOk);
     return;
   }
 
   // Make sure we are notified if http://crbug.com/49709 returns.  Issue also
   // occurs with some damaged files.
   if (input->timestamp() == kNoTimestamp()) {
     DLOG(ERROR) << "Received a buffer without timestamps!";
     decode_cb.Run(kDecodeError);
     return;
   }
 
-  // Apply the necessary codec delay.
-  if (start_input_timestamp_ == kNoTimestamp())

[chcunningham, 2015/07/31 19:48:44]

Am I right that what you removed here is equivalent behavior to what we have
now? That is, we used to set start_timestamp with the TS from the first buffer
and configure the discard_helper with preskip. Now we just we configure the
discard helper as soon as the configuration arrives. The effect in both cases is
that we discard the first so many buffers we see (and adjust timestamps) until
preskip is satisfied. If I'm right, the new way is definitely cleaner, but it
makes me wonder if I've missed something. The change description suggests to me
that the original behavior was different/wrong.

[DaleCurtis, 2015/08/03 22:49:53]

Kind of, in the first playback case the behavior is similar. As noted in the
description, however subsequent playbacks or seeks would do the wrong thing and
be initialized with the seek preroll instead of the codec delay though.

-    start_input_timestamp_ = input->timestamp();
-  if (!discard_helper_->initialized() &&
-      input->timestamp() == start_input_timestamp_) {
-    discard_helper_->Reset(config_.codec_delay());
-  }
-
   scoped_refptr<AudioBuffer> output_buffer;
 
   if (!Decode(input, &output_buffer)) {
     decode_cb.Run(kDecodeError);
     return;
   }
 
   if (output_buffer.get()) {
     output_cb_.Run(output_buffer);
   }
@@ skipping 73 matching lines @@
   }
 
   status = opus_multistream_decoder_ctl(
       opus_decoder_, OPUS_SET_GAIN(opus_extra_data.gain_db));
   if (status != OPUS_OK) {
     DLOG(ERROR) << "Failed to set OPUS header gain; status="
                 << opus_strerror(status);
     return false;
   }
 
-  discard_helper_.reset(
-      new AudioDiscardHelper(config_.samples_per_second(), 0));
-  start_input_timestamp_ = kNoTimestamp();
+  ResetTimestampState();
   return true;
 }
 
 void OpusAudioDecoder::CloseDecoder() {
   if (opus_decoder_) {
     opus_multistream_decoder_destroy(opus_decoder_);
-    opus_decoder_ = NULL;
+    opus_decoder_ = nullptr;
   }
 }
 
 void OpusAudioDecoder::ResetTimestampState() {
-  discard_helper_->Reset(
-      discard_helper_->TimeDeltaToFrames(config_.seek_preroll()));
+  discard_helper_.reset(
+      new AudioDiscardHelper(config_.samples_per_second(), 0));
+  discard_helper_->Reset(config_.codec_delay());
 }
 
 bool OpusAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& input,
                               scoped_refptr<AudioBuffer>* output_buffer) {
   // Allocate a buffer for the output samples.
   *output_buffer = AudioBuffer::CreateBuffer(
       config_.sample_format(),
       config_.channel_layout(),
       ChannelLayoutToChannelCount(config_.channel_layout()),
       config_.samples_per_second(),
@@ skipping 23 matching lines @@
   }
 
   // Trim off any extraneous allocation.
   DCHECK_LE(frames_decoded, output_buffer->get()->frame_count());
   const int trim_frames = output_buffer->get()->frame_count() - frames_decoded;
   if (trim_frames > 0)
     output_buffer->get()->TrimEnd(trim_frames);
 
   // Handles discards and timestamping.  Discard the buffer if more data needed.
   if (!discard_helper_->ProcessBuffers(input, *output_buffer))
-    *output_buffer = NULL;
+    *output_buffer = nullptr;
 
   return true;
 }
 
 }  // namespace media