Parsing of encoded duration for unencrypted opus streams.

media/formats/webm/webm_cluster_parser.h

 // Copyright 2014 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_FORMATS_WEBM_WEBM_CLUSTER_PARSER_H_
 #define MEDIA_FORMATS_WEBM_WEBM_CLUSTER_PARSER_H_
 
 #include <deque>
 #include <map>
 #include <set>
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
+#include "media/base/audio_decoder_config.h"
 #include "media/base/media_export.h"
 #include "media/base/media_log.h"
 #include "media/base/stream_parser.h"
 #include "media/base/stream_parser_buffer.h"
 #include "media/formats/webm/webm_parser.h"
 #include "media/formats/webm/webm_tracks_parser.h"
 
 namespace media {
 
 class MEDIA_EXPORT WebMClusterParser : public WebMParserClient {
  public:
   typedef StreamParser::TrackId TrackId;
   typedef std::deque<scoped_refptr<StreamParserBuffer> > BufferQueue;
   typedef std::map<TrackId, const BufferQueue> TextBufferQueueMap;
 
   // Arbitrarily-chosen numbers to estimate the duration of a buffer if none is
   // set and there is not enough information to get a better estimate.
   // TODO(wolenetz/acolwell): Parse audio codebook to determine missing audio

[wolenetz, 2015/01/30 01:48:08]

nit: update bug comment :) (wolenetz/chcunningham), vorbis...

[chcunningham, 2015/02/03 20:34:43]

Done. I pulled the TODO out of this comment and put the TODO you proposed inside
the impl for TryGetEncodedAudioDuration. I think its a little more intuitive
when placed there. For e.g., when I come back to implement Vorbis, I won't
really be touching these estimation defaults at all, but I will be changing the
impl of TryGetEncodedAudioDuration.

   // frame durations. See http://crbug.com/351166.
   enum {
     kDefaultAudioBufferDurationInMs = 23,  // Common 1k samples @44.1kHz
     kDefaultVideoBufferDurationInMs = 42  // Low 24fps to reduce stalls
   };
 
+  // Opus packets encode the duration and other parameters in the 5 most

[wolenetz, 2015/01/30 01:48:08]

move to impl. drop last ','

[chcunningham, 2015/02/03 20:34:43]

Done.

+  // significant bits of the first byte. The index in this array corresponds
+  // to the duration of each frame of the packet in microseconds. See
+  // http://goo.gl/2RmoxA
+  const uint16 kOpusFrameDurationsMu[32] = {
+      10000, 20000, 40000, 60000, 10000, 20000, 40000, 60000,
+      10000, 20000, 40000, 60000, 10000, 20000, 10000, 20000,
+      2500,  5000,  10000, 20000, 2500,  5000,  10000, 20000,
+      2500,  5000,  10000, 20000, 2500,  5000,  10000, 20000,
+  };
+
  private:
   // Helper class that manages per-track state.
   class Track {
    public:
     Track(int track_num,
           bool is_video,
           base::TimeDelta default_duration,
           const LogCB& log_cb);
     ~Track();
 
@@ skipping 66 matching lines @@
 
     // Buffers in (decode) timestamp order that were previously parsed into and
     // extracted from |buffers_|. Buffers are moved from |buffers_| to
     // |ready_buffers_| by ExtractReadyBuffers() if they are below a specified
     // upper bound timestamp. Track users can therefore extract only those
     // parsed buffers which are "ready" for emission (all before some maximum
     // timestamp).
     BufferQueue ready_buffers_;
 
     // If kNoTimestamp(), then |estimated_next_frame_duration_| will be used.
     base::TimeDelta default_duration_;

[wolenetz, 2015/01/30 01:48:08]

nit: Hmm. it seems the comments for how estimation vs extraction vs default vs
hardcode are split up all over. Maybe document at top or have a helper method
that handles *all* the duration extraction/fallback estimation/hardcoding logic
and put the comments there and refer to that documentation here and in
|estimated_next_frame_duration|? This is getting harder and harder to read.

[chcunningham, 2015/02/03 20:34:43]

I really pondered this (and we chatted). I think a centralized comment is only
better if we can also centralize the duration calc/estimation code. Right now we
have fragmentation across these 4 places...

1. ClusterParser::OnBlock
	1. encoded duration
	2. block duration
	3. track default duration

2. Track::AddBuffer
	1. set delta duration on *previous* buffer when needed
	2. mark this buffer as needing delta if it doesn't have one

3. Track::ApplyDurationEstimateIfNeeded
	1. Last buffer missing duration? Set from GetDurationEstimate

4. Track::GetDurationEstimate
	1. first try's to use estimated_next_frame_duration (min)
	2. fallback to hardcoded


The conservative approach:
Each of these 4 places has a good comment in the .h file. Perhaps the only thing
that's missing is a comment in OnBlock impl that mentions how estimation is
handled in end of cluster Scenario. I've added a little blurb there. 

A possibility for consolidation:
We could move the calling of estimation code (3 & 4) into OnBlock, but only if
we can know that the Block we're parsing is the last in the cluster. That info
is known by the underlying list parser, which has a "done" state. Right now it
only enters the "done" state *AFTER* OnBlock is called. *BUT* we could make it
transition just before calling OnBlock. To make it less subtle, we could
additionally add a flag to OnListEnd (which calls OnBlock, passing the flag
along) to indicate that this list (Block) is the last in the cluster. What do
you think? It would allow us to completely remove the
ApplyDurationEstimateIfNeeded in the Get{Audio/Video}Buffers path. I say we go
for it, but want you to call out the things I'm probably overlooking ;)

[wolenetz, 2015/02/03 22:47:01]

Refactoring the estimation stuff to occur in OnBlock is too big for inclusion in
this CL. Please file a P3 bug to track this potential clean-up.

 
     // If kNoTimestamp(), then a default value will be used. This estimate is
     // the maximum duration seen or derived so far for this track, and is valid
     // only if |default_duration_| is kNoTimestamp().
     base::TimeDelta estimated_next_frame_duration_;
 
     LogCB log_cb_;
   };
 
   typedef std::map<int, Track> TextTrackMap;
 
  public:
   WebMClusterParser(int64 timecode_scale,
                     int audio_track_num,
                     base::TimeDelta audio_default_duration,
                     int video_track_num,
                     base::TimeDelta video_default_duration,
                     const WebMTracksParser::TextTracks& text_tracks,
                     const std::set<int64>& ignored_tracks,
                     const std::string& audio_encryption_key_id,
                     const std::string& video_encryption_key_id,
+                    const AudioDecoderConfig& audio_config,
                     const LogCB& log_cb);
   ~WebMClusterParser() override;
 
   // Resets the parser state so it can accept a new cluster.
   void Reset();
 
   // Parses a WebM cluster element in |buf|.
   //
   // Returns -1 if the parse fails.
   // Returns 0 if more data is needed.
@@ skipping 64 matching lines @@
   // bound.)
   // Parse() or Reset() must be called between calls to UpdateReadyBuffers() to
   // clear each track's ready buffers and to reset |ready_buffer_upper_bound_|
   // to kNoDecodeTimestamp().
   void UpdateReadyBuffers();
 
   // Search for the indicated track_num among the text tracks.  Returns NULL
   // if that track num is not a text track.
   Track* FindTextTrack(int track_num);
 
+  // Attempts to read the duration from the encoded audio data, storing output
+  // in |duration| in microseconds. This obviously violates layering rules, but
+  // is useful for MSE to know duration in cases where it isn't explicitly given
+  // and cannot be calculated when for Blocks at the end of a Cluster (The next

[wolenetz, 2015/01/30 01:48:08]

nits: grammar (when for), un-cap (The

[chcunningham, 2015/02/03 20:34:43]

Done.

+  // Cluster in playback-order may not be the next Cluster we parse, so we can't
+  // simply use the delta of the first Block in the next Cluster). Avoid calling
+  // if encrypted; may produce unexpected output. See implementation for
+  // supported codecs.
+  bool TryGetEncodedAudioDuration(const uint8* data, int size, int64* duration);

[wolenetz, 2015/01/30 01:48:08]

What is bool retval?

[chcunningham, 2015/02/03 20:34:43]

Now base::TimeDelta

+  // Reads Opus packet header to determine packet duration.

[wolenetz, 2015/01/30 01:48:08]

nit: insert blank link prior.

[chcunningham, 2015/02/03 20:34:43]

Done.

+  bool ReadOpusDuration(const uint8* data, int size, int64* duration);
+
   double timecode_multiplier_;  // Multiplier used to convert timecodes into
                                 // microseconds.
   std::set<int64> ignored_tracks_;
   std::string audio_encryption_key_id_;
   std::string video_encryption_key_id_;
+  const AudioDecoderConfig& audio_config_;

[wolenetz, 2015/01/30 01:48:08]

Do we need the whole config? Or just some specific info like codec id?
Also, I don't like refs-as-members in general (and I don't see any usage of
refs-as-members elsewhere in chromium). If you need a non-copy of some class,
consider smart pointers. Lifetime, backing-store, etc issues... refs-as-members
sound like a path to eventual pain ;)

[chcunningham, 2015/02/03 20:34:43]

Done. Now just taking in the codec.

 
   WebMListParser parser_;
 
   int64 last_block_timecode_;
   scoped_ptr<uint8[]> block_data_;
   int block_data_size_;
   int64 block_duration_;
   int64 block_add_id_;
 
   scoped_ptr<uint8[]> block_additional_data_;
@@ skipping 26 matching lines @@
   DecodeTimestamp ready_buffer_upper_bound_;
 
   LogCB log_cb_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(WebMClusterParser);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_FORMATS_WEBM_WEBM_CLUSTER_PARSER_H_