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

Issue 530243002: Move common VideoPacket initialization into VideoEncoderHelper. (Closed)

Created:
6 years, 3 months ago by Wez
Modified:
6 years, 3 months ago
Reviewers:
Sergey Ulanov
CC:
chromium-reviews, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Move common VideoPacket initialization into VideoEncoderHelper. This avoids duplication of common VideoPacket initialization across multiple encoders, making addition of new fields (e.g. the recently added shape field) more robust. Committed: https://crrev.com/59ed1bb4dcd82cf212e7a9b3c27e3d533e25ad2e Cr-Commit-Position: refs/heads/master@{#293064}

Patch Set 1 #

Patch Set 2 : Add unit tests #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+251 lines, -52 lines) Patch
A remoting/codec/video_encoder_helper.h View 1 chunk +46 lines, -0 lines 0 comments Download
A remoting/codec/video_encoder_helper.cc View 1 chunk +70 lines, -0 lines 0 comments Download
A remoting/codec/video_encoder_helper_unittest.cc View 1 1 chunk +107 lines, -0 lines 0 comments Download
M remoting/codec/video_encoder_verbatim.h View 2 chunks +3 lines, -7 lines 0 comments Download
M remoting/codec/video_encoder_verbatim.cc View 4 chunks +13 lines, -27 lines 0 comments Download
M remoting/codec/video_encoder_vpx.h View 2 chunks +4 lines, -0 lines 0 comments Download
M remoting/codec/video_encoder_vpx.cc View 2 chunks +4 lines, -18 lines 1 comment Download
M remoting/host/video_frame_recorder_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/remoting_srcs.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M remoting/remoting_test.gypi View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
Wez
PTAL
6 years, 3 months ago (2014-09-03 00:29:00 UTC) #2
Sergey Ulanov
lgtm https://codereview.chromium.org/530243002/diff/20001/remoting/codec/video_encoder_vpx.cc File remoting/codec/video_encoder_vpx.cc (right): https://codereview.chromium.org/530243002/diff/20001/remoting/codec/video_encoder_vpx.cc#newcode289 remoting/codec/video_encoder_vpx.cc:289: // TODO(hclam): Make sure we get exactly one ...
6 years, 3 months ago (2014-09-03 01:11:26 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wez@chromium.org/530243002/20001
6 years, 3 months ago (2014-09-03 01:16:34 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001) as 08a848ed5b18a4fdcc5a2645f78048dc9f9972c9
6 years, 3 months ago (2014-09-03 04:37:46 UTC) #6
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:23:20 UTC) #7
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/59ed1bb4dcd82cf212e7a9b3c27e3d533e25ad2e
Cr-Commit-Position: refs/heads/master@{#293064}

Powered by Google App Engine
This is Rietveld 408576698