Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(14)

Issue 372943002: Add video frame recording capability to Chromoting hosts. (Closed)

Created:
5 years, 4 months ago by Wez
Modified:
5 years, 3 months ago
Reviewers:
Sergey Ulanov, Jamie
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, chromoting-reviews_chromium.org, Jamie
Project:
chromium
Visibility:
Public.

Description

Add video frame recording capability to Chromoting hosts. This will be used to record sequences of video frames for real-world sessions, for performance-evaluation purposes. If the host is run with a video-recording buffer size specified then it will advertise a videoRecorder capability to clients. Clients seeing this capability can send "video-recorder" extension messages to the host to start and stop recording, and to fetch the next recorded frame. Frames are encoded to VideoPackets using the "verbatim" encoder, and must then be serialized and Base-64 encoded for transmission via the extension message protocol. See crrev.com/386853002 for the client part of this. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287183

Patch Set 1 #

Patch Set 2 : Working implementation. #

Total comments: 13

Patch Set 3 : Rebase on HostExtensionSessionManager #

Patch Set 4 : Update to use ModifiesVideoPipeline(). #

Patch Set 5 : Rebase #

Patch Set 6 : Correct comment #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+274 lines, -6 lines) Patch
M remoting/host/chromoting_host.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M remoting/host/host_config.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M remoting/host/host_config.cc View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/host/remoting_me2me_host.cc View 1 2 7 chunks +32 lines, -1 line 0 comments Download
M remoting/host/video_frame_recorder.h View 1 2 2 chunks +7 lines, -2 lines 2 comments Download
M remoting/host/video_frame_recorder.cc View 1 2 2 chunks +24 lines, -2 lines 0 comments Download
A remoting/host/video_frame_recorder_host_extension.h View 1 2 1 chunk +38 lines, -0 lines 2 comments Download
A remoting/host/video_frame_recorder_host_extension.cc View 1 2 3 4 1 chunk +161 lines, -0 lines 4 comments Download
M remoting/remoting_host.gypi View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M remoting/webapp/client_plugin.js View 1 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/webapp/client_session.js View 1 2 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
Wez
PTAL This approach ends up being considerably less efficient than adding a new message to ...
5 years, 4 months ago (2014-07-11 02:28:46 UTC) #1
Sergey Ulanov
https://codereview.chromium.org/372943002/diff/20001/remoting/host/chromoting_host.cc File remoting/host/chromoting_host.cc (right): https://codereview.chromium.org/372943002/diff/20001/remoting/host/chromoting_host.cc#newcode187 remoting/host/chromoting_host.cc:187: if (!enable) { nit: remove {} https://codereview.chromium.org/372943002/diff/20001/remoting/host/chromoting_host.h File remoting/host/chromoting_host.h ...
5 years, 4 months ago (2014-07-11 18:38:04 UTC) #2
Wez
Jamie, PTAL! https://codereview.chromium.org/372943002/diff/20001/remoting/host/remoting_me2me_host.cc File remoting/host/remoting_me2me_host.cc (right): https://codereview.chromium.org/372943002/diff/20001/remoting/host/remoting_me2me_host.cc#newcode128 remoting/host/remoting_me2me_host.cc:128: const char kFrameRecorderBufferKbName[] = "frame-recorder-buffer-kb"; On 2014/07/11 ...
5 years, 3 months ago (2014-07-31 23:57:46 UTC) #3
Jamie
You seem to be sending the video stream twice when the recording feature is enabled. ...
5 years, 3 months ago (2014-08-01 19:53:52 UTC) #4
Wez
The aim of this extension is to capture pristeen frames, with no encoding artefacts, from ...
5 years, 3 months ago (2014-08-01 22:33:56 UTC) #5
Wez
The CQ bit was checked by wez@chromium.org
5 years, 3 months ago (2014-08-01 22:34:04 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wez@chromium.org/372943002/140001
5 years, 3 months ago (2014-08-01 22:36:33 UTC) #7
commit-bot: I haz the power
5 years, 3 months ago (2014-08-02 07:55:54 UTC) #8
Message was sent while issue was closed.
Change committed as 287183

Powered by Google App Engine
This is Rietveld 408576698