Add status parameter to DemuxerStream::ReadCB

media/base/demuxer_stream.h

 // 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.
 
 #ifndef MEDIA_BASE_DEMUXER_STREAM_H_
 #define MEDIA_BASE_DEMUXER_STREAM_H_
 
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "media/base/media_export.h"
 #include "media/base/ranges.h"
 
 namespace media {
 
 class AudioDecoderConfig;
 class DecoderBuffer;
 class VideoDecoderConfig;
 
 class MEDIA_EXPORT DemuxerStream
     : public base::RefCountedThreadSafe<DemuxerStream> {
  public:
   enum Type {
     UNKNOWN,
     AUDIO,
     VIDEO,
     NUM_TYPES,  // Always keep this entry as the last one!
   };
 
+  enum Status {

[Ami GONE FROM CHROMIUM, 2012/06/26 00:33:21]

Before this CL, ReadCB had 3 possible return scenarios (normal, EOS, abort). 
After this CL, it has 4 possible return scenarios (above + config change).  Does
it not make sense to promote EOS to a Status now that you must pass/inspect
Status anyway?

[acolwell GONE FROM CHROMIUM, 2012/07/12 01:19:38]

I believe it does make sense. Mind if I defer this to a follow-on CL?

+    kOk,
+    kAborted,
+    kConfigChanged,
+  };
+
   // Request a buffer to returned via the provided callback.
   //
-  // Non-NULL buffer pointers will contain media data or signal the end of the
-  // stream. A NULL pointer indicates an aborted Read(). This can happen if the
-  // DemuxerStream gets flushed and doesn't have any more data to return.
-  typedef base::Callback<void(const scoped_refptr<DecoderBuffer>&)> ReadCB;
+  // The first parameter indicates the status of the read.
+  //  kOk : Indicates the second parameter is Non-NULL and contains media data

[Ami GONE FROM CHROMIUM, 2012/06/26 00:33:21]

These comments belong at the enum declaration, not here.

[acolwell GONE FROM CHROMIUM, 2012/07/12 01:19:38]

Done.

+  //        or the end of the stream.
+  //  kAborted : Indicates an aborted Read(). This can happen if the
+  //             DemuxerStream gets flushed and doesn't have any more data to
+  //             return. The second parameter MUST be NULL when this status is
+  //             return.

[Ami GONE FROM CHROMIUM, 2012/06/26 00:33:21]

s/\./ed./

[acolwell GONE FROM CHROMIUM, 2012/07/12 01:19:38]

Done.

+  //  kConfigChange : Indicates that the decoder configuration has changed and a
+  //                  call to audio_decoder_config() or video_decoder_config()

[Ami GONE FROM CHROMIUM, 2012/06/26 00:33:21]

This phrasing implies that calling [av]_d_c() on a demuxer stream will
reconfigure a decoder or something.  Rephrase?

[acolwell GONE FROM CHROMIUM, 2012/07/12 01:19:38]

Done.

+  //                  is needed before more data is returned. The second
+  //                  parameter MUST be NULL when this status is return.

[Ami GONE FROM CHROMIUM, 2012/06/26 00:33:21]

ditto

[acolwell GONE FROM CHROMIUM, 2012/07/12 01:19:38]

Done.

+  typedef base::Callback<void(Status, const scoped_refptr<DecoderBuffer>&)>
+      ReadCB;
   virtual void Read(const ReadCB& read_cb) = 0;
 
   // Returns the audio decoder configuration. It is an error to call this method
   // if type() != AUDIO.
   virtual const AudioDecoderConfig& audio_decoder_config() = 0;
 
   // Returns the video decoder configuration. It is an error to call this method
   // if type() != VIDEO.
   virtual const VideoDecoderConfig& video_decoder_config() = 0;
 
   // Returns time ranges known to have been seen by this stream.
   virtual Ranges<base::TimeDelta> GetBufferedRanges() = 0;
 
   // Returns the type of stream.
   virtual Type type() = 0;
 
   virtual void EnableBitstreamConverter() = 0;
 
  protected:
   friend class base::RefCountedThreadSafe<DemuxerStream>;
   virtual ~DemuxerStream();
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_DEMUXER_STREAM_H_