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

Issue 1996453003: RTC Video Encoder: Use capturer timestamp (Closed)

Created:
4 years, 8 months ago by shenghao
Modified:
4 years, 7 months ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, imcheng+watch_chromium.org, posciak+watch_chromium.org, avayvod+watch_chromium.org, jam, feature-media-reviews_chromium.org, darin-cc_chromium.org, jasonroberts+watch_google.com, mkwst+moarreviews-renderer_chromium.org, piman+watch_chromium.org, xjz+watch_chromium.org, isheriff+watch_chromium.org, miu+watch_chromium.org, mflodman_chromium_OOO, wuchengli
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

RTC Video Encoder: Use capturer timestamp Add the capturer timestamp as a parameter in VEA::BitstreamBufferReady() so RTCVideoEncoder can use it. BUG=350106 TEST=Verify that apprtc loopback works on minnie. R=bbudge@chromium.org, posciak@chromium.org, sandersd@chromium.org, wfh@chromium.org, wuchengli@chromium.org Committed: https://crrev.com/5d967288829b022d533d839e4883902858615682 Cr-Commit-Position: refs/heads/master@{#396786}

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : #

Total comments: 30

Patch Set 4 : #

Patch Set 5 : #

Total comments: 8

Patch Set 6 : #

Patch Set 7 : #

Total comments: 8

Patch Set 8 : #

Patch Set 9 : rebase #

Patch Set 10 : #

Patch Set 11 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -44 lines) Patch
M content/renderer/media/rtc_video_encoder.cc View 1 2 3 4 5 chunks +10 lines, -11 lines 0 comments Download
M content/renderer/media/video_track_recorder.cc View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -2 lines 0 comments Download
M content/renderer/pepper/pepper_video_encoder_host.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/pepper/pepper_video_encoder_host.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -2 lines 0 comments Download
M content/renderer/pepper/video_encoder_shim.cc View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M media/cast/sender/external_video_encoder.cc View 1 chunk +2 lines, -1 line 0 comments Download
M media/gpu/android_video_encode_accelerator.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M media/gpu/ipc/client/gpu_video_encode_accelerator_host.h View 1 chunk +2 lines, -1 line 0 comments Download
M media/gpu/ipc/client/gpu_video_encode_accelerator_host.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M media/gpu/ipc/common/media_messages.h View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -2 lines 0 comments Download
M media/gpu/ipc/service/gpu_video_encode_accelerator.h View 1 chunk +2 lines, -1 line 0 comments Download
M media/gpu/ipc/service/gpu_video_encode_accelerator.cc View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -2 lines 0 comments Download
M media/gpu/v4l2_video_encode_accelerator.cc View 1 2 3 4 5 6 7 8 2 chunks +10 lines, -2 lines 0 comments Download
M media/gpu/vaapi_video_encode_accelerator.h View 1 2 3 4 5 6 7 2 chunks +3 lines, -1 line 0 comments Download
M media/gpu/vaapi_video_encode_accelerator.cc View 1 2 3 4 5 6 4 chunks +7 lines, -4 lines 0 comments Download
M media/gpu/video_encode_accelerator_unittest.cc View 1 2 3 4 5 6 7 8 9 10 5 chunks +12 lines, -2 lines 0 comments Download
M media/gpu/vt_video_encode_accelerator_mac.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +6 lines, -4 lines 0 comments Download
M media/video/fake_video_encode_accelerator.cc View 1 2 3 4 5 6 1 chunk +2 lines, -3 lines 0 comments Download
M media/video/video_encode_accelerator.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 39 (16 generated)
bbudge
content/renderer/pepper lgtm https://codereview.chromium.org/1996453003/diff/1/content/renderer/pepper/video_encoder_shim.cc File content/renderer/pepper/video_encoder_shim.cc (right): https://codereview.chromium.org/1996453003/diff/1/content/renderer/pepper/video_encoder_shim.cc#newcode490 content/renderer/pepper/video_encoder_shim.cc:490: // TODO: Pass real timestamp. TODO(llandwerlin)
4 years, 8 months ago (2016-05-19 09:29:50 UTC) #1
shenghao
On 2016/05/19 09:29:50, bbudge wrote: > content/renderer/pepper lgtm > > https://codereview.chromium.org/1996453003/diff/1/content/renderer/pepper/video_encoder_shim.cc > File content/renderer/pepper/video_encoder_shim.cc (right): ...
4 years, 8 months ago (2016-05-19 09:57:11 UTC) #2
Will Harris
media/gpu/ipc/common/media_messages.h lgtm
4 years, 8 months ago (2016-05-19 09:57:57 UTC) #3
sandersd (OOO until July 31)
media/ LGTM % documentation nit. https://codereview.chromium.org/1996453003/diff/40001/media/video/video_encode_accelerator.h File media/video/video_encode_accelerator.h (right): https://codereview.chromium.org/1996453003/diff/40001/media/video/video_encode_accelerator.h#newcode81 media/video/video_encode_accelerator.h:81: // |key_frame| is true ...
4 years, 8 months ago (2016-05-19 20:27:12 UTC) #7
pbos
drive-by of RtcVideoEncoder from a WebRTC perspective. We need the 90khz RTP timestamp that WebRTC ...
4 years, 8 months ago (2016-05-20 12:15:51 UTC) #9
pbos
Oh, and thanks a lot for taking care of this. We've been wanting this change ...
4 years, 8 months ago (2016-05-20 12:17:56 UTC) #10
wuchengli
Removing myself from reviewers. Leaving this to Pawel.
4 years, 8 months ago (2016-05-23 02:54:57 UTC) #11
Pawel Osciak
Please test this using a new test case in veaunittest as proposed in my comment ...
4 years, 8 months ago (2016-05-23 07:15:53 UTC) #13
shenghao
On 2016/05/23 07:15:53, Pawel Osciak wrote: > Please test this using a new test case ...
4 years, 8 months ago (2016-05-24 10:48:00 UTC) #14
shenghao
https://codereview.chromium.org/1996453003/diff/1/content/renderer/pepper/video_encoder_shim.cc File content/renderer/pepper/video_encoder_shim.cc (right): https://codereview.chromium.org/1996453003/diff/1/content/renderer/pepper/video_encoder_shim.cc#newcode490 content/renderer/pepper/video_encoder_shim.cc:490: // TODO: Pass real timestamp. On 2016/05/19 09:29:49, bbudge ...
4 years, 8 months ago (2016-05-24 10:50:20 UTC) #15
pbos
lgtm, thanks a lot
4 years, 8 months ago (2016-05-24 13:02:21 UTC) #16
pbos
On 2016/05/24 13:02:21, pbos wrote: > lgtm, thanks a lot (I only checked RTCVideoEncoder.)
4 years, 8 months ago (2016-05-24 13:02:36 UTC) #17
Pawel Osciak
https://chromiumcodereview.appspot.com/1996453003/diff/40001/media/gpu/v4l2_video_encode_accelerator.cc File media/gpu/v4l2_video_encode_accelerator.cc (right): https://chromiumcodereview.appspot.com/1996453003/diff/40001/media/gpu/v4l2_video_encode_accelerator.cc#newcode669 media/gpu/v4l2_video_encode_accelerator.cc:669: static_cast<int64_t>(dqbuf.timestamp.tv_usec)))); On 2016/05/24 10:50:19, shenghao wrote: > On 2016/05/23 ...
4 years, 8 months ago (2016-05-26 07:39:24 UTC) #18
shenghao
https://codereview.chromium.org/1996453003/diff/40001/media/gpu/v4l2_video_encode_accelerator.cc File media/gpu/v4l2_video_encode_accelerator.cc (right): https://codereview.chromium.org/1996453003/diff/40001/media/gpu/v4l2_video_encode_accelerator.cc#newcode669 media/gpu/v4l2_video_encode_accelerator.cc:669: static_cast<int64_t>(dqbuf.timestamp.tv_usec)))); On 2016/05/26 07:39:23, Pawel Osciak wrote: > On ...
4 years, 8 months ago (2016-05-26 10:38:20 UTC) #19
Pawel Osciak
lgtm % nits please also, please update the TEST= line https://codereview.chromium.org/1996453003/diff/120001/media/gpu/v4l2_video_encode_accelerator.cc File media/gpu/v4l2_video_encode_accelerator.cc (right): https://codereview.chromium.org/1996453003/diff/120001/media/gpu/v4l2_video_encode_accelerator.cc#newcode669 ...
4 years, 8 months ago (2016-05-27 04:46:39 UTC) #20
shenghao
https://codereview.chromium.org/1996453003/diff/120001/media/gpu/v4l2_video_encode_accelerator.cc File media/gpu/v4l2_video_encode_accelerator.cc (right): https://codereview.chromium.org/1996453003/diff/120001/media/gpu/v4l2_video_encode_accelerator.cc#newcode669 media/gpu/v4l2_video_encode_accelerator.cc:669: base::TimeDelta::FromMicroseconds( On 2016/05/27 04:46:38, Pawel Osciak wrote: > Perhaps ...
4 years, 8 months ago (2016-05-30 09:18:02 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996453003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996453003/140001
4 years, 8 months ago (2016-05-30 09:18:24 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-generic_chromium_compile_only_ng/builds/144998)
4 years, 8 months ago (2016-05-30 09:28:51 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996453003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996453003/160001
4 years, 8 months ago (2016-05-30 10:13:05 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: android_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_compile_dbg/builds/73697)
4 years, 8 months ago (2016-05-30 10:24:54 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996453003/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996453003/200001
4 years, 8 months ago (2016-05-31 03:43:24 UTC) #34
commit-bot: I haz the power
Committed patchset #11 (id:200001)
4 years, 8 months ago (2016-05-31 04:59:50 UTC) #36
commit-bot: I haz the power
4 years, 8 months ago (2016-05-31 05:02:24 UTC) #38
Message was sent while issue was closed.
Patchset 11 (id:??) landed as
https://crrev.com/5d967288829b022d533d839e4883902858615682
Cr-Commit-Position: refs/heads/master@{#396786}

Powered by Google App Engine
This is Rietveld 408576698