Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1976)

Unified Diff: content/renderer/media/video_track_recorder.cc

Issue 1313603004: MediaRecorderHandler (video part) and unittests (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase, gyp and MediaRecorderHandlerTest Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/renderer/media/video_track_recorder.cc
diff --git a/content/renderer/media/video_track_recorder.cc b/content/renderer/media/video_track_recorder.cc
index b5aaa72e0b1ab43fe8f68c94946f626ed889271f..0636177e5d4adc3f6a5a144a91df2075597a279e 100644
--- a/content/renderer/media/video_track_recorder.cc
+++ b/content/renderer/media/video_track_recorder.cc
@@ -10,8 +10,6 @@
#include "base/threading/thread.h"
#include "base/time/time.h"
#include "base/trace_event/trace_event.h"
-#include "content/child/child_process.h"
-#include "media/base/bind_to_current_loop.h"
#include "media/base/video_frame.h"
extern "C" {
@@ -28,16 +26,31 @@ using media::VideoFrameMetadata;
namespace content {
namespace {
+
const vpx_codec_flags_t kNoFlags = 0;
+// Originally from remoting/codec/scoped_vpx_codec.h.
+// TODO(mcasas): Refactor into a common location.
+struct VpxCodecDeleter {
+ void operator()(vpx_codec_ctx_t* codec) {
+ if (!codec)
+ return;
+ vpx_codec_err_t ret = vpx_codec_destroy(codec);
+ CHECK_EQ(ret, VPX_CODEC_OK) << "Failed to destroy codec";
miu 2015/09/02 21:28:13 nit: No need for the logging string.
mcasas 2015/09/04 02:16:30 Done.
+ delete codec;
+ }
+};
+
+typedef scoped_ptr<vpx_codec_ctx_t, VpxCodecDeleter> ScopedVpxCodec;
+
void OnFrameEncodeCompleted(
- const content::VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_cb,
+ const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_cb,
const scoped_refptr<VideoFrame>& frame,
scoped_ptr<std::string> data,
base::TimeTicks capture_timestamp,
bool keyframe) {
- DVLOG(1) << (keyframe ? "" : "non ") << "keyframe "
- << capture_timestamp << " ms - " << data->length() << "B ";
+ DVLOG(1) << (keyframe ? "" : "non ") << "keyframe "<< data->length() << "B, "
+ << capture_timestamp << " ms";
on_encoded_video_cb.Run(frame, base::StringPiece(*data), capture_timestamp,
keyframe);
}
@@ -45,23 +58,29 @@ void OnFrameEncodeCompleted(
} // anonymous namespace
// Inner class encapsulating all libvpx interactions and the encoding+delivery
-// of received frames. This class is:
-// - created and destroyed on its parent's thread (usually the main render
-// thread),
-// - receives VideoFrames and Run()s the callbacks on another thread (supposedly
-// the render IO thread), which is cached on first frame arrival,
+// of received frames. Limitation: Only VP8 is supported for the time being.
+// This class needs to be ref-counted to avoid destruction (of |encoder_| in
+// particular) while there's still a frame being processed in either IO or
+// encode threads. This class:
+// - is created and destroyed on its parent's thread, usually the main Render
+// thread;
+// - receives VideoFrames and Run()s the callbacks on |origin_task_runner_|,
+// which is cached on first frame arrival, usually this thread is to be the
+// render IO thread, but this is not enforced;
// - uses an internal |encoding_thread_| for libvpx interactions, notably for
// encoding (which might take some time).
-// Only VP8 is supported for the time being.
-class VideoTrackRecorder::VpxEncoder final {
+class VideoTrackRecorder::VpxEncoder final
+ : public base::RefCountedThreadSafe<VpxEncoder> {
public:
explicit VpxEncoder(const OnEncodedVideoCB& on_encoded_video_callback);
- ~VpxEncoder();
void StartFrameEncode(const scoped_refptr<VideoFrame>& frame,
base::TimeTicks capture_timestamp);
private:
+ friend class base::RefCountedThreadSafe<VpxEncoder>;
+ ~VpxEncoder();
+
void EncodeOnEncodingThread(const scoped_refptr<VideoFrame>& frame,
base::TimeTicks capture_timestamp);
@@ -86,9 +105,10 @@ class VideoTrackRecorder::VpxEncoder final {
// Thread for encoding. Active as long as VpxEncoder exists. All variables
// below this are used in this thread.
base::Thread encoding_thread_;
- // VP8 internal objects: configuration, encoder and Vpx Image wrapper.
+ // VP8 internal objects: configuration and encoder.
vpx_codec_enc_cfg_t codec_config_;
- vpx_codec_ctx_t encoder_;
+ // |encoder_| is a special scoped pointer to guarantee proper destruction.
+ ScopedVpxCodec encoder_;
// The |VideoFrame::timestamp()| of the last encoded frame. This is used to
// predict the duration of the next frame.
@@ -113,7 +133,6 @@ VideoTrackRecorder::VpxEncoder::~VpxEncoder() {
DCHECK(main_render_thread_checker_.CalledOnValidThread());
miu 2015/09/02 21:28:13 Because this is now ref-counted, there is no guara
mcasas 2015/09/04 02:16:30 You're right. I've checked other MediaStreamVideo
miu 2015/09/04 21:48:52 Seem to me that you can write a dtor that is able
mcasas 2015/09/05 02:43:01 Done.
DCHECK(encoding_thread_.IsRunning());
encoding_thread_.Stop();
- vpx_codec_destroy(&encoder_);
}
void VideoTrackRecorder::VpxEncoder::StartFrameEncode(
@@ -126,7 +145,7 @@ void VideoTrackRecorder::VpxEncoder::StartFrameEncode(
encoding_thread_.task_runner()->PostTask(
FROM_HERE, base::Bind(&VpxEncoder::EncodeOnEncodingThread,
- base::Unretained(this), frame, capture_timestamp));
+ this, frame, capture_timestamp));
}
void VideoTrackRecorder::VpxEncoder::EncodeOnEncodingThread(
@@ -161,21 +180,21 @@ void VideoTrackRecorder::VpxEncoder::EncodeOnEncodingThread(
// Encode the frame. The presentation time stamp argument here is fixed to
// zero to force the encoder to base its single-frame bandwidth calculations
// entirely on |predicted_frame_duration|.
- const vpx_codec_err_t ret = vpx_codec_encode(&encoder_,
+ const vpx_codec_err_t ret = vpx_codec_encode(encoder_.get(),
&vpx_image,
0 /* pts */,
duration.InMicroseconds(),
kNoFlags,
VPX_DL_REALTIME);
DCHECK_EQ(ret, VPX_CODEC_OK) << vpx_codec_err_to_string(ret) << ", #"
- << vpx_codec_error(&encoder_) << " -"
- << vpx_codec_error_detail(&encoder_);
+ << vpx_codec_error(encoder_.get()) << " -"
+ << vpx_codec_error_detail(encoder_.get());
scoped_ptr<std::string> data(new std::string);
bool keyframe = false;
vpx_codec_iter_t iter = NULL;
const vpx_codec_cx_pkt_t* pkt = NULL;
- while ((pkt = vpx_codec_get_cx_data(&encoder_, &iter)) != NULL) {
+ while ((pkt = vpx_codec_get_cx_data(encoder_.get(), &iter)) != NULL) {
if (pkt->kind != VPX_CODEC_CX_FRAME_PKT)
continue;
data->assign(static_cast<char*>(pkt->data.frame.buf), pkt->data.frame.sz);
@@ -183,7 +202,7 @@ void VideoTrackRecorder::VpxEncoder::EncodeOnEncodingThread(
break;
}
origin_task_runner_->PostTask(FROM_HERE,
- base::Bind(&OnFrameEncodeCompleted,
+ base::Bind(OnFrameEncodeCompleted,
on_encoded_video_callback_,
frame,
base::Passed(&data),
@@ -200,7 +219,7 @@ void VideoTrackRecorder::VpxEncoder::ConfigureVp8Encoding(
DVLOG(1) << "Destroying/Re-Creating encoder for new frame size: "
<< gfx::Size(codec_config_.g_w, codec_config_.g_h).ToString()
<< " --> " << size.ToString();
- vpx_codec_destroy(&encoder_);
+ encoder_.reset();
}
const vpx_codec_iface_t* interface = vpx_codec_vp8_cx();
@@ -246,7 +265,9 @@ void VideoTrackRecorder::VpxEncoder::ConfigureVp8Encoding(
// Number of frames to consume before producing output.
codec_config_.g_lag_in_frames = 0;
- const vpx_codec_err_t ret = vpx_codec_enc_init(&encoder_, interface,
+ DCHECK(!encoder_);
+ encoder_.reset(new vpx_codec_ctx_t);
miu 2015/09/02 21:28:13 Why did you change this to a custom scoped ptr? I
mcasas 2015/09/04 02:16:30 Is due to the possible case of ctor() + dtor() wit
miu 2015/09/04 21:48:52 But you have IsInitialized() to tell you whether y
mcasas 2015/09/05 02:43:01 Acknowledged.
+ const vpx_codec_err_t ret = vpx_codec_enc_init(encoder_.get(), interface,
&codec_config_, kNoFlags);
DCHECK_EQ(VPX_CODEC_OK, ret);
}
@@ -285,28 +306,27 @@ VideoTrackRecorder::VideoTrackRecorder(
const blink::WebMediaStreamTrack& track,
const OnEncodedVideoCB& on_encoded_video_callback)
: track_(track),
- encoder_(new VpxEncoder(on_encoded_video_callback)),
- weak_factory_(this) {
+ encoder_(new VpxEncoder(on_encoded_video_callback)) {
DCHECK(main_render_thread_checker_.CalledOnValidThread());
DCHECK(!track_.isNull());
DCHECK(track.extraData());
+
+ // StartFrameEncode() will be called on Render IO thread.
AddToVideoTrack(this,
- media::BindToCurrentLoop(
- base::Bind(&VideoTrackRecorder::OnVideoFrame,
- weak_factory_.GetWeakPtr())),
+ base::Bind(&VideoTrackRecorder::VpxEncoder::StartFrameEncode,
+ encoder_),
track_);
}
VideoTrackRecorder::~VideoTrackRecorder() {
DCHECK(main_render_thread_checker_.CalledOnValidThread());
RemoveFromVideoTrack(this, track_);
- weak_factory_.InvalidateWeakPtrs();
track_.reset();
}
-void VideoTrackRecorder::OnVideoFrame(const scoped_refptr<VideoFrame>& frame,
- base::TimeTicks timestamp) {
- DCHECK(main_render_thread_checker_.CalledOnValidThread());
+void VideoTrackRecorder::OnVideoFrameForTesting(
+ const scoped_refptr<media::VideoFrame>& frame,
+ base::TimeTicks timestamp) {
encoder_->StartFrameEncode(frame, timestamp);
}

Powered by Google App Engine
This is Rietveld 408576698