Cleanup a bunch of stale expectations and style around empty packets.

Our decoders ignore zero-byte packets, so there's no reason to send
them out of the demuxer. They should just be dropped at the time of
demuxing. Also cleans up weird .get()-> syntax from yesteryear.

BUG=none
TEST=existing test pass.

Cq-Include-Trybots: luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I99ddd10d3156cecb3a5edd6f3b940d96530f3184
Reviewed-on: https://chromium-review.googlesource.com/965393
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543847}

media/filters/ffmpeg_audio_decoder.cc

@@ -163,11 +163,8 @@ bool FFmpegAudioDecoder::FFmpegDecode(
     packet.data = const_cast<uint8_t*>(buffer->data());
     packet.size = buffer->data_size();
 
-    // Since we're not at EOS and there is no data available in the current
-    // buffer, simply return and let the caller provide more data.
-    // crbug.com/663438 has more context on 0-byte buffers.
-    if (!packet.size)
-      return true;
+    DCHECK(packet.data);
+    DCHECK_GT(packet.size, 0);
   }
 
   bool decoded_frame_this_loop = false;

media/filters/ffmpeg_demuxer.cc

@@ -362,6 +362,8 @@ FFmpegDemuxerStream::~FFmpegDemuxerStream() {
 
 void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(packet->size);
+  DCHECK(packet->data);
 
   if (!demuxer_ || end_of_stream_) {
     NOTREACHED() << "Attempted to enqueue packet on a stopped stream";
@@ -369,7 +371,7 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
   }
 
   if (waiting_for_keyframe_) {
-    if (packet.get()->flags & AV_PKT_FLAG_KEY)
+    if (packet->flags & AV_PKT_FLAG_KEY)
       waiting_for_keyframe_ = false;
     else {
       DVLOG(3) << "Dropped non-keyframe pts=" << packet->pts;
@@ -379,9 +381,9 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
 
 #if BUILDFLAG(USE_PROPRIETARY_CODECS)
   // Convert the packet if there is a bitstream filter.
-  if (packet->data && bitstream_converter_ &&
+  if (bitstream_converter_ &&
       !bitstream_converter_->ConvertPacket(packet.get())) {
-    LOG(ERROR) << "Format conversion failed.";
+    MEDIA_LOG(ERROR, media_log_) << "Format conversion failed.";
   }
 #endif
 
@@ -403,7 +405,7 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
                  settings_data, settings_data + settings_size,
                  &side_data);
 
-    buffer = DecoderBuffer::CopyFrom(packet.get()->data, packet.get()->size,
+    buffer = DecoderBuffer::CopyFrom(packet->data, packet->size,
                                      side_data.data(), side_data.size());
   } else {
     int side_data_size = 0;
@@ -418,7 +420,7 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
               packet->data, packet->size,
               reinterpret_cast<const uint8_t*>(encryption_key_id_.data()),
               encryption_key_id_.size(), &decrypt_config, &data_offset)) {
-        LOG(ERROR) << "Creation of DecryptConfig failed.";
+        MEDIA_LOG(ERROR, media_log_) << "Creation of DecryptConfig failed.";
       }
     }
 
@@ -426,12 +428,12 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
     // reference inner memory of FFmpeg.  As such we should transfer the packet
     // into memory we control.
     if (side_data_size > 0) {
-      buffer = DecoderBuffer::CopyFrom(packet.get()->data + data_offset,
-                                       packet.get()->size - data_offset,
-                                       side_data, side_data_size);
+      buffer = DecoderBuffer::CopyFrom(packet->data + data_offset,
+                                       packet->size - data_offset, side_data,
+                                       side_data_size);
     } else {
-      buffer = DecoderBuffer::CopyFrom(packet.get()->data + data_offset,
-                                       packet.get()->size - data_offset);
+      buffer = DecoderBuffer::CopyFrom(packet->data + data_offset,
+                                       packet->size - data_offset);
     }
 
     int skip_samples_size = 0;
@@ -581,7 +583,7 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
     }
   }
 
-  if (packet.get()->flags & AV_PKT_FLAG_KEY)
+  if (packet->flags & AV_PKT_FLAG_KEY)
     buffer->set_is_key_frame(true);
 
   last_packet_timestamp_ = buffer->timestamp();
@@ -1839,29 +1841,22 @@ void FFmpegDemuxer::OnReadFrameDone(ScopedAVPacket packet, int result) {
   // Queue the packet with the appropriate stream; we must defend against ffmpeg
   // giving us a bad stream index.  See http://crbug.com/698549 for example.
   if (packet->stream_index >= 0 &&
-      packet->stream_index < static_cast<int>(streams_.size()) &&
-      streams_[packet->stream_index]) {
-    // TODO(scherkus): Fix demuxing upstream to never return packets w/o data
-    // when av_read_frame() returns success code. See bug comment for ideas:
-    //
-    // https://code.google.com/p/chromium/issues/detail?id=169133#c10
-    if (!packet->data) {
-      ScopedAVPacket new_packet(new AVPacket());
-      av_new_packet(new_packet.get(), 0);
-      av_packet_copy_props(new_packet.get(), packet.get());
-      packet.swap(new_packet);
-    }
-
-    FFmpegDemuxerStream* demuxer_stream = streams_[packet->stream_index].get();
-    if (demuxer_stream->IsEnabled())
-      demuxer_stream->EnqueuePacket(std::move(packet));
-
-    // If duration estimate was incorrect, update it and tell higher layers.
-    if (duration_known_) {
-      const base::TimeDelta duration = demuxer_stream->duration();
-      if (duration != kNoTimestamp && duration > duration_) {
-        duration_ = duration;
-        host_->SetDuration(duration_);
+      static_cast<size_t>(packet->stream_index) < streams_.size()) {
+    // Drop empty packets since they're ignored on the decoder side anyways.
+    if (!packet->data || !packet->size) {
+      DLOG(WARNING) << "Dropping empty packet, size: " << packet->size
+                    << ", data: " << static_cast<void*>(packet->data);
+    } else if (auto& demuxer_stream = streams_[packet->stream_index]) {
+      if (demuxer_stream->IsEnabled())
+        demuxer_stream->EnqueuePacket(std::move(packet));
+
+      // If duration estimate was incorrect, update it and tell higher layers.
+      if (duration_known_) {
+        const base::TimeDelta duration = demuxer_stream->duration();
+        if (duration != kNoTimestamp && duration > duration_) {
+          duration_ = duration;
+          host_->SetDuration(duration_);
+        }
       }
     }
   }

media/filters/ffmpeg_video_decoder.cc

@@ -335,12 +335,8 @@ bool FFmpegVideoDecoder::FFmpegDecode(
     packet.data = const_cast<uint8_t*>(buffer->data());
     packet.size = buffer->data_size();
 
-    // Starting in M60, the decoder generates an error if an empty buffer is
-    // provided. Since we're not at EOS and there is no data available in the
-    // current buffer, simply return and let the caller provide more data.
-    // crbug.com/663438 has more context on 0-byte buffers.
-    if (!packet.size)
-      return true;
+    DCHECK(packet.data);
+    DCHECK_GT(packet.size, 0);
 
     // Let FFmpeg handle presentation timestamp reordering.
     codec_context_->reordered_opaque = buffer->timestamp().InMicroseconds();

media/filters/ffmpeg_video_decoder_unittest.cc

@@ -261,25 +261,6 @@ TEST_F(FFmpegVideoDecoderTest, DecodeFrame_Normal) {
   ASSERT_EQ(1U, output_frames_.size());
 }
 
-// Verify current behavior for 0 byte frames. FFmpegVideoDecoder simply ignores
-// the 0 byte frames.
-TEST_F(FFmpegVideoDecoderTest, DecodeFrame_0ByteFrame) {
-  Initialize();
-
-  scoped_refptr<DecoderBuffer> zero_byte_buffer = new DecoderBuffer(0);
-
-  InputBuffers input_buffers;
-  input_buffers.push_back(i_frame_buffer_);
-  input_buffers.push_back(zero_byte_buffer);
-  input_buffers.push_back(i_frame_buffer_);
-  input_buffers.push_back(end_of_stream_buffer_);
-
-  DecodeStatus status = DecodeMultipleFrames(input_buffers);
-
-  EXPECT_EQ(DecodeStatus::OK, status);
-  ASSERT_EQ(2U, output_frames_.size());
-}
-
 TEST_F(FFmpegVideoDecoderTest, DecodeFrame_DecodeError) {
   Initialize();
 

media/gpu/ipc/service/gpu_video_decode_accelerator.cc

@@ -464,8 +464,7 @@ void GpuVideoDecodeAccelerator::OnAssignPictureBuffers(
 
           // TODO(dshwang): after moving to D3D11, remove this.
           // https://crbug.com/438691
-          GLenum format =
-              video_decode_accelerator_.get()->GetSurfaceInternalFormat();
+          GLenum format = video_decode_accelerator_->GetSurfaceInternalFormat();
           if (format != GL_RGBA) {
             DCHECK(format == GL_BGRA_EXT);
             texture_manager->SetLevelInfo(texture_ref, texture_target_, 0,