Motown: Improvements to packet definition

services/media/framework_ffmpeg/ffmpeg_audio_decoder.cc

 // Copyright 2016 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 "base/logging.h"
 #include "services/media/framework_ffmpeg/ffmpeg_audio_decoder.h"
 
 namespace mojo {
 namespace media {
 
@@ skipping 10 matching lines @@
     // Prepare for interleaving.
     stream_type_ = output_stream_type();
     lpcm_util_ = LpcmUtil::Create(*stream_type_->lpcm());
   }
 }
 
 FfmpegAudioDecoder::~FfmpegAudioDecoder() {}
 
 void FfmpegAudioDecoder::Flush() {
   FfmpegDecoderBase::Flush();
-  next_presentation_time_= Packet::kUnknownPresentationTime;
+  next_pts_= Packet::kUnknownPts;
 }
 
 int FfmpegAudioDecoder::Decode(
     const AVPacket& av_packet,
     const AvFramePtr& av_frame_ptr,
     PayloadAllocator* allocator,
     bool* frame_decoded_out) {
   DCHECK(allocator);
   DCHECK(frame_decoded_out);
   DCHECK(context());
   DCHECK(av_frame_ptr);
 
-  if (next_presentation_time_ == Packet::kUnknownPresentationTime) {
+  if (next_pts_ == Packet::kUnknownPts) {
     if (av_packet.pts == AV_NOPTS_VALUE) {
-      next_presentation_time_ = 0;
+      next_pts_ = 0;

[johngro, 2016/03/21 19:47:56]

I'm still a bit confused by this logic; eg, why is the ffmpeg decoder wrapper
making timestamp interpolation decisions instead of just putting the timestamps
produced by ffmpeg onto the payloads during CreateOutputPacket?

No action is being requested here.  Just noting that it is something we need to
sit down and discuss at some point.

[dalesat, 2016/03/21 22:30:59]

Acknowledged.

     } else {
-      next_presentation_time_ = av_packet.pts;
+      next_pts_ = av_packet.pts;
     }
   }
 
   // Use the provided allocator (for allocations in AllocateBufferForAvFrame)
   // unless we intend to interleave later, in which case use the default
   // allocator. We'll interleave into a buffer from the provided allocator
   // in CreateOutputPacket.
   allocator_ = lpcm_util_ ? PayloadAllocator::GetDefault() : allocator;
 
   int frame_decoded = 0;
   int input_bytes_used = avcodec_decode_audio4(
       context().get(),
       av_frame_ptr.get(),
       &frame_decoded,
       &av_packet);
   *frame_decoded_out = frame_decoded != 0;
 
   // We're done with this allocator.
   allocator_ = nullptr;
 
   return input_bytes_used;
 }
 
 PacketPtr FfmpegAudioDecoder::CreateOutputPacket(
     const AVFrame& av_frame,
     PayloadAllocator* allocator) {
   DCHECK(allocator);
 
-  int64_t presentation_time = av_frame.pts;
-  if (presentation_time == AV_NOPTS_VALUE) {
-    presentation_time = next_presentation_time_;
-    next_presentation_time_ += av_frame.nb_samples;
+  int64_t pts = av_frame.pts;
+  if (pts == AV_NOPTS_VALUE) {
+    pts = next_pts_;
+    next_pts_ += av_frame.nb_samples;
   }
 
   uint64_t payload_size;
   void *payload_buffer;
 
   AvBufferContext* av_buffer_context =
       reinterpret_cast<AvBufferContext*>(av_buffer_get_opaque(av_frame.buf[0]));
 
   if (lpcm_util_) {
     // We need to interleave. The non-interleaved frames are in a buffer that
@@ skipping 12 matching lines @@
         av_frame.nb_samples);
   } else {
     // We don't need to interleave. The interleaved frames are in a buffer that
     // was allocated from the correct allocator. We take ownership of the buffer
     // by calling Release here so that ReleaseBufferForAvFrame won't release it.
     payload_size = av_buffer_context->size();
     payload_buffer = av_buffer_context->Release();
   }
 
   return Packet::Create(
-      presentation_time,
-      av_frame.nb_samples,
+      pts,
       false, // The base class is responsible for end-of-stream.
       payload_size,
       payload_buffer,
       allocator);
 }
 
 PacketPtr FfmpegAudioDecoder::CreateOutputEndOfStreamPacket() {
-  return Packet::CreateEndOfStream(next_presentation_time_);
+  return Packet::CreateEndOfStream(next_pts_);
 }
 
 int FfmpegAudioDecoder::AllocateBufferForAvFrame(
     AVCodecContext* av_codec_context,
     AVFrame* av_frame,
     int flags) {
   // CODEC_CAP_DR1 is required in order to do allocation this way.
   DCHECK(av_codec_context->codec->capabilities & CODEC_CAP_DR1);
 
   FfmpegAudioDecoder* self =
@@ skipping 78 matching lines @@
   // Either this buffer has already been released to someone else's ownership,
   // or it's the same as the buffer parameter.
   DCHECK(
       av_buffer_context->buffer() == nullptr ||
       av_buffer_context->buffer() == buffer);
   delete av_buffer_context;
 }
 
 } // namespace media
 } // namespace mojo