Avoid unnecessary post task during frame delivery.

media/base/video_decoder.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_VIDEO_DECODER_H_
 #define MEDIA_BASE_VIDEO_DECODER_H_
 
 #include <string>
 
 #include "base/callback.h"
@@ skipping 47 matching lines @@
   // satisfy the requirements above.
   //
   // |cdm_context| can be used to handle encrypted buffers. May be null if the
   // stream is not encrypted.
   //
   // Note:
   // 1) The VideoDecoder will be reinitialized if it was initialized before.
   //    Upon reinitialization, all internal buffered frames will be dropped.
   // 2) This method should not be called during pending decode or reset.
   // 3) No VideoDecoder calls should be made before |init_cb| is executed.
+  // 4) VideoDecoders should take care to run |output_cb| as soon as the frame
+  // is ready (i.e. w/o thread trampolining) since it can strongly affect frame
+  // delivery times with high-frame-rate material.  See Decode() for additional
+  // notes.
   virtual void Initialize(const VideoDecoderConfig& config,
                           bool low_delay,
                           CdmContext* cdm_context,
                           const InitCB& init_cb,
                           const OutputCB& output_cb) = 0;
 
   // Requests a |buffer| to be decoded. The status of the decoder and decoded
   // frame are returned via the provided callback. Some decoders may allow
   // decoding multiple buffers in parallel. Callers should call
   // GetMaxDecodeRequests() to get number of buffers that may be decoded in
   // parallel. Decoder must call |decode_cb| in the same order in which Decode()
   // is called.
   //
   // Implementations guarantee that the callback will not be called from within
   // this method and that |decode_cb| will not be blocked on the following
   // Decode() calls (i.e. |decode_cb| will be called even if Decode() is never
   // called again).
   //
   // After decoding is finished the decoder calls |output_cb| specified in
-  // Initialize() for each decoded frame. In general |output_cb| may be called
-  // before or after |decode_cb|, but software decoders normally call
-  // |output_cb| before calling |decode_cb|, i.e. while Decode() is pending.
+  // Initialize() for each decoded frame. |output_cb| is always called before
+  // |decode_cb|. However, |output_cb| may be called before Decode() returns, if
+  // other behavior is desired callers should ensure that |output_cb| will
+  // trampoline as necessary.

[xhwang, 2016/04/25 22:40:28]

Note that we requires that the |decode_cb| must NOT be called from within this
method. If the caller trampolines/posts the output_cb and still want to make
sure output_cb is called before decode_cb, then it needs to post the decode_cb
again. This makes me a bit sad. Maybe we should just say something like "the
calls must handles this case correctly".

[DaleCurtis, 2016/04/25 22:52:16]

Do we need to say anything? If |output_cb| runs while Decode() is in flight, it
will execute before |decode_cb| is posted. I don't think anything else is
necessary but a BindToCurrentLoop.

[DaleCurtis, 2016/04/25 23:07:43]

Discussed offline, while the existing behavior is preserved, the use of "may" in
the comment would cause issues when blindly using BTCL -- so that comment has
been removed in favor of the simpler: "callers must handle this case." Which is
already the case everywhere.

   //
   // If |buffer| is an EOS buffer then the decoder must be flushed, i.e.
   // |output_cb| must be called for each frame pending in the queue and
   // |decode_cb| must be called after that. Callers will not call Decode()
   // again until after the flush completes.
   virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer,
                       const DecodeCB& decode_cb) = 0;
 
   // Resets decoder state. All pending Decode() requests will be finished or
   // aborted before |closure| is called.
@@ skipping 12 matching lines @@
   // Returns maximum number of parallel decode requests.
   virtual int GetMaxDecodeRequests() const;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(VideoDecoder);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_VIDEO_DECODER_H_