Define EncodedVideoSource and RtcCapturedEncodingVideoCapturer.

content/renderer/media/rtc_encoding_video_capturer.cc

+// Copyright (c) 2013 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 "content/renderer/media/rtc_encoding_video_capturer.h"
+
+#include "media/base/encoded_bitstream_buffer.h"
+
+namespace content {
+
+// Wrapper to for EncodedVideoBitstream that retains known state information
+// for incoming bitstream.
+class RtcEncodingVideoCapturer::Bitstream :
+    public media::EncodedVideoBitstream::Client,

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

This implies the name of the class is poor - it's a Bitstream's Client, not a
Bitstream itself.

[hshi1, 2013/06/10 21:49:35]

How about "BitstreamClient"? See updated CL.

+    public base::RefCountedThreadSafe<RtcEncodingVideoCapturer::Bitstream> {

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

why refcount?

[hshi1, 2013/06/10 21:49:35]

Refcount is not really needed, I just need it to be a scoped ptr owned by the
RtcEncodingVideoCapturer. Would that work?

+ public:
+  Bitstream(media::VideoEncodingParameters params,
+            media::EncodedVideoSource* encoded_video_source);

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

doco lifetime

+
+  // Function to configure runtime encoding parameters.

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Isn't this really just the EVB interface?

[hshi1, 2013/06/10 21:49:35]

... yes, what was I thinking. This is completely bogus. Removed.

+  void Configure(const media::RuntimeVideoEncodingParameters& params);

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Try?

[hshi1, 2013/06/10 21:49:35]

This is no longer needed.

+
+  // media::EncodedVideoBitstream::Client implementation.
+  virtual void OnBitstreamStreaming(
+      scoped_refptr<media::EncodedVideoBitstream> bitstream,
+      const media::VideoEncodingParameters& params) OVERRIDE;
+  virtual void OnBitstreamRemoved(
+      scoped_refptr<media::EncodedVideoBitstream> bitstream) OVERRIDE;
+  virtual void OnBitstreamReady(
+      scoped_refptr<media::EncodedVideoBitstream> bitstream,
+      scoped_refptr<const media::EncodedBitstreamBuffer> buffer) OVERRIDE;
+  virtual void OnBitstreamConfigChanged(
+      scoped_refptr<media::EncodedVideoBitstream> bitstream,
+      const media::RuntimeVideoEncodingParameters& params) OVERRIDE;
+
+  // Getters and setters for bitstream properties.
+  bool finished() const;
+  media::RuntimeVideoEncodingParameters runtime_params() const;
+  void set_round_trip_time(int rtt_in_ms);

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

use TimeDelta for the param

[hshi1, 2013/06/10 19:12:31]

Done.

+  void set_callback(webrtc::EncodedImageCallback* callback);
+
+ private:
+  virtual ~Bitstream();
+  friend class base::RefCountedThreadSafe<RtcEncodingVideoCapturer::Bitstream>;
+
+  // Helper to convert buffer to webrtc types in callback.
+  bool Convert(scoped_refptr<const media::EncodedBitstreamBuffer> buffer,
+               webrtc::EncodedImage& image,
+               webrtc::CodecSpecificInfo& codecInfo,
+               webrtc::RTPFragmentationHeader& fragHeader);
+
+  media::VideoEncodingParameters params_;
+  bool finished_;
+
+  // Representing incoming stream.
+  scoped_refptr<media::EncodedVideoBitstream> bitstream_;
+
+  base::Time time_base_;
+  base::TimeDelta round_trip_time_;
+  media::EncodedVideoSource* encoded_video_source_;
+  webrtc::EncodedImageCallback* callback_;
+  DISALLOW_COPY_AND_ASSIGN(Bitstream);

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

nit: extra \n before this

[hshi1, 2013/06/10 19:12:31]

Done.

+};
+
+RtcEncodingVideoCapturer::Bitstream::Bitstream(
+    media::VideoEncodingParameters params,
+    media::EncodedVideoSource* encoded_video_source)
+    : params_(params),
+      finished_(false),
+      encoded_video_source_(encoded_video_source) {

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

initialize all other members to known-reasonable sentinels (0, NULL, etc)

[hshi1, 2013/06/10 21:49:35]

Done.

+  if (encoded_video_source_)

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

how can this fail?

[hshi1, 2013/06/10 21:49:35]

Done.

+    bitstream_ = encoded_video_source_->OpenBitstream(this, params);
+}
+
+RtcEncodingVideoCapturer::Bitstream::~Bitstream() {
+  if (encoded_video_source_ && bitstream_) {

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

how can this fail?

[hshi1, 2013/06/10 21:49:35]

Well encoded_video_source_ should be valid, but bitstream_ can still be NULL if
OpenBitstream failed, or if bitstream is already closed. Updated here.

+    encoded_video_source_->CloseBitstream(bitstream_);
+    bitstream_ = NULL;

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

why bother?

[hshi1, 2013/06/10 21:49:35]

Done.

+  }
+}
+
+void RtcEncodingVideoCapturer::Bitstream::Configure(
+    const media::RuntimeVideoEncodingParameters& params) {
+  if (bitstream_)

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

how can this fail?

[hshi1, 2013/06/10 21:49:35]

Can fail if OpenBitstream failed, or bitstream is already closed.

+    bitstream_->TryConfigure(params);
+}
+
+bool RtcEncodingVideoCapturer::Bitstream::finished() const {
+  return finished_;
+}
+
+media::RuntimeVideoEncodingParameters
+    RtcEncodingVideoCapturer::Bitstream::runtime_params() const {
+  return params_.runtime_params;
+}
+
+void RtcEncodingVideoCapturer::Bitstream::set_round_trip_time(int rtt_in_ms) {
+  round_trip_time_ = base::TimeDelta::FromMilliseconds(rtt_in_ms);
+}
+
+void RtcEncodingVideoCapturer::Bitstream::set_callback(
+    webrtc::EncodedImageCallback* callback) {
+  callback_ = callback;

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

DCHECK(!callback_);
first?

[hshi1, 2013/06/10 21:49:35]

Done.

+}
+
+void RtcEncodingVideoCapturer::Bitstream::OnBitstreamStreaming(
+    scoped_refptr<media::EncodedVideoBitstream> bitstream,
+    const media::VideoEncodingParameters& params) {
+  params_ = params;
+}
+
+void RtcEncodingVideoCapturer::Bitstream::OnBitstreamRemoved(
+    scoped_refptr<media::EncodedVideoBitstream> bitstream) {
+  finished_ = true;

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

bitstream_ = NULL;
?

[hshi1, 2013/06/10 21:49:35]

Done.

+}
+
+void RtcEncodingVideoCapturer::Bitstream::OnBitstreamReady(
+    scoped_refptr<media::EncodedVideoBitstream> bitstream,
+    scoped_refptr<const media::EncodedBitstreamBuffer> buffer) {
+  if (callback_) {
+    // First buffer constitutes the origin of the time for this bitstream

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Isn't this true regardless of callback_ being set?

[hshi1, 2013/06/10 21:49:35]

Yes but if callback_ is not set then we can't call callback_->Encoded. I've
moved the check to just before invoking callback_->Encoded.

+    // context.
+    if (time_base_.is_null()) {
+      time_base_ = buffer->metadata().timestamp;
+    }
+    // Convert EncodedBitstreamBuffer to webrtc types.
+    webrtc::EncodedImage image;
+    webrtc::CodecSpecificInfo codecInfo;
+    webrtc::RTPFragmentationHeader fragInfo;
+    if (Convert(buffer, image, codecInfo, fragInfo)) {

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

style: no non-const ref params.  These are output params and so should be passed
by pointer.
But really it seems strange to pass in output params just to use them in the
very next line.
Wouldn't it be simpler/better to move the Encoded() call into Convert() (after
renaming it)?

[hshi1, 2013/06/10 21:49:35]

Done.

+      callback_->Encoded(image, &codecInfo, &fragInfo);
+    }
+  }
+  // Return buffer so it can be reused for encoding upcoming bitstream.
+  if (encoded_video_source_)

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

can never be false.

[hshi1, 2013/06/10 21:49:35]

Done.

+    encoded_video_source_->ReturnBitstreamBuffer(bitstream, buffer);
+}

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

This collection of comments reduces l.120-136 into:
if (time_base_.is_null())
  time_base_ = buffer->metadata().timestamp;
ReportEncodedFrame(buffer);
encoded_video_source_->ReturnBitstreamBuffer(bitstream, buffer);

and ReportEncodedFrame is Convert with an early-return if !callback_ and a final
callback_->Encoded() call at its bottom.

[hshi1, 2013/06/10 21:49:35]

Done.

+
+void RtcEncodingVideoCapturer::Bitstream::OnBitstreamConfigChanged(
+    scoped_refptr<media::EncodedVideoBitstream> bitstream,
+    const media::RuntimeVideoEncodingParameters& params) {
+  params_.runtime_params = params;
+}
+
+bool RtcEncodingVideoCapturer::Bitstream::Convert(

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

return value is ignored at only callsite and no legit return false codepaths so
s/bool/void/ I think.

[hshi1, 2013/06/10 21:49:35]

Made it void.

+    scoped_refptr<const media::EncodedBitstreamBuffer> buffer,
+    webrtc::EncodedImage& image,
+    webrtc::CodecSpecificInfo& codecInfo,
+    webrtc::RTPFragmentationHeader& fragHeader) {
+  DCHECK(buffer);
+  if (!buffer) {
+    return false;
+  }
+
+  // TODO: remove this const_cast. Unfortunately webrtc::EncodedImage defines

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

TODO's should always be attributed like TODO(hshi)
(please make a pass)

[hshi1, 2013/06/10 21:49:35]

Done.

+  // member |_buffer| of type uint8_t* even though webrtc never modifies the
+  // buffer contents.
+  image._buffer = const_cast<uint8_t*>(buffer->buffer());
+  image._length = buffer->size();
+  image._size = image._length;

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Do you know what the diff is supposed to be between _size and _length?

[hshi1, 2013/06/10 21:49:35]

|_size| is supposed to indicate buffer capacity, and |_length| the amount of
valid bitstream data (in bytes). For our situation they're always identical.

+
+  const media::BufferEncodingMetadata& metadata = buffer->metadata();
+  image.capture_time_ms_ =
+      (metadata.timestamp - time_base_).InMilliseconds(); // 1 KHz

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Comment doesn't make sense to me.

[hshi1, 2013/06/10 21:49:35]

Done.

+  image._timeStamp = image.capture_time_ms_ * 90; // 90kHz

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Once we finalize this CL IWBN to have mflodman@ or another webrtc-eng confirm
this is Doing It Right(tm).

[hshi1, 2013/06/10 21:49:35]

I agree that this is not accurate; please review this new version.

+  if (metadata.key_frame) {
+    image._frameType = webrtc::kKeyFrame;
+  } else {
+    image._frameType = webrtc::kDeltaFrame;
+  }
+  image._completeFrame = true;
+  image._encodedWidth = params_.resolution.width();
+  image._encodedHeight = params_.resolution.height();
+
+  // TODO: generate codec specific info for VP8.
+  codecInfo.codecType = webrtc::kVideoCodecGeneric;
+
+  // Generate header containing a single fragmentation.
+  fragHeader.VerifyAndAllocateFragmentationHeader(1);
+  fragHeader.fragmentationOffset[0] = 0;
+  fragHeader.fragmentationLength[0] = buffer->size();
+  fragHeader.fragmentationPlType[0] = 0;
+  fragHeader.fragmentationTimeDiff[0] = 0;
+
+  return true;
+}
+
+// RtcEncodingVideoCapturer
+RtcEncodingVideoCapturer::RtcEncodingVideoCapturer(
+    int stream_id,
+    media::EncodedVideoSource* encoded_video_source)
+    : stream_id_(stream_id),
+      encoded_video_source_(encoded_video_source) {
+}
+
+RtcEncodingVideoCapturer::~RtcEncodingVideoCapturer() {
+}
+
+int32_t RtcEncodingVideoCapturer::InitEncode(
+    const webrtc::VideoCodec* codecSettings,
+    int32_t numberOfCores,
+    uint32_t maxPayloadSize) {
+  DCHECK(!bitstream_);
+  if (!bitstream_) {

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

How can this fail?
(and if you wanted to handle InitEncode being called twice, you probably
wouldn't want to return OK)

[hshi1, 2013/06/10 21:49:35]

Done.

+    // Convert |codecSettings| to |params|.
+    media::VideoEncodingParameters params;
+    params.codec_name = codecSettings->plName;
+    params.resolution = gfx::Size(codecSettings->width, codecSettings->height);
+    params.frames_per_second = codecSettings->maxFramerate;
+    params.runtime_params.average_bitrate = codecSettings->maxBitrate;

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

s/max/start/ ?

[hshi1, 2013/06/10 19:12:31]

Done.

+    params.runtime_params.max_bitrate = codecSettings->maxBitrate;
+    bitstream_ = new Bitstream(params, encoded_video_source_);
+  }
+  return WEBRTC_VIDEO_CODEC_OK;
+}
+
+int32_t RtcEncodingVideoCapturer::Encode(
+    const webrtc::I420VideoFrame& inputImage,
+    const webrtc::CodecSpecificInfo* codecSpecificInfo,
+    const std::vector<webrtc::VideoFrameType>* frame_types) {
+  // TODO: request specific frame type.

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

TODO(hshi)

[hshi1, 2013/06/10 19:12:31]

Done.

+  return WEBRTC_VIDEO_CODEC_OK;
+}
+
+int32_t RtcEncodingVideoCapturer::RegisterEncodeCompleteCallback(
+      webrtc::EncodedImageCallback* callback) {
+  if (!bitstream_)
+    return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
+  bitstream_->set_callback(callback);

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Are you really not guaranteed that RECC is called _after_ InitEncode?  Given
that they're all synchronous, this paranoia seems unwarranted.
(if indeed not necessary, then a bunch of code simplifies as
Bitstream::set_callback can go into Bitstream's ctor, etc)

[hshi1, 2013/06/10 21:49:35]

You're right.

+  return WEBRTC_VIDEO_CODEC_OK;
+}
+
+int32_t RtcEncodingVideoCapturer::Release() {
+  DCHECK(bitstream_);
+  bitstream_ = NULL;

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

Are you relying on this moving the refcount to 0?
Wouldn't it be better to 
bitstream_->OnBitstreamRemoved();
before 
bitstream_ = NULL;
?

+  return WEBRTC_VIDEO_CODEC_OK;
+}
+
+int32_t RtcEncodingVideoCapturer::SetChannelParameters(
+    uint32_t packetLoss,

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

/* packetLoss */ here & in the header

[hshi1, 2013/06/10 19:12:31]

Done.

+    int rtt_in_ms) {
+  if (!bitstream_)
+    return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
+  bitstream_->set_round_trip_time(rtt_in_ms);
+  return WEBRTC_VIDEO_CODEC_OK;
+}
+
+int32_t RtcEncodingVideoCapturer::SetRates(uint32_t newBitRate,
+                                           uint32_t frameRate) {
+  if (!bitstream_)
+    return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
+  // TODO: wire up runtime rate control.

[Ami GONE FROM CHROMIUM, 2013/06/08 00:18:01]

TODO(hshi)

[hshi1, 2013/06/10 19:12:31]

Done.

+  return WEBRTC_VIDEO_CODEC_OK;
+}
+
+}  // namespace content