|
|
Created:
3 years, 9 months ago by Hzj_jie Modified:
3 years, 9 months ago Reviewers:
Sergey Ulanov CC:
chromium-reviews, posciak+watch_chromium.org, chromoting-reviews_chromium.org, Jamie Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[Chromoting] Send DesktopFrame::capturer_id() to client through WebrtcVideoStream
Once DirectX Capturer is enabled or being experimented, we can use the
capturer_id() logged in clients to analyze the failure rate.
BUG=650926, 679523
Review-Url: https://codereview.chromium.org/2767193007
Cr-Original-Commit-Position: refs/heads/master@{#459625}
Committed: https://chromium.googlesource.com/chromium/src/+/f478d76ee3acd92b6a2cf5ff2e5a0e4094c756db
Review-Url: https://codereview.chromium.org/2767193007
Cr-Commit-Position: refs/heads/master@{#459684}
Committed: https://chromium.googlesource.com/chromium/src/+/805f2ca5aa0902f56885ea3c8c0a42cb80d84522
Patch Set 1 #
Total comments: 2
Patch Set 2 : Resolve review comments #Patch Set 3 : crash in msan #
Messages
Total messages: 33 (25 generated)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org
LGTM after my comment is addressed. https://codereview.chromium.org/2767193007/diff/20001/remoting/protocol/webrt... File remoting/protocol/webrtc_video_stream.cc (right): https://codereview.chromium.org/2767193007/diff/20001/remoting/protocol/webrt... remoting/protocol/webrtc_video_stream.cc:44: uint32_t capturer_id; I think it's better to put this in WebrtcVideoStream::FrameTimestamps. And then rename FrameTimestamps -> FrameStats and EncodedFrameWithTimestamps -> EncodedFrameWithStats. Also please initialize this field.
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2767193007/diff/20001/remoting/protocol/webrt... File remoting/protocol/webrtc_video_stream.cc (right): https://codereview.chromium.org/2767193007/diff/20001/remoting/protocol/webrt... remoting/protocol/webrtc_video_stream.cc:44: uint32_t capturer_id; On 2017/03/24 23:54:26, Sergey Ulanov wrote: > I think it's better to put this in WebrtcVideoStream::FrameTimestamps. > And then rename FrameTimestamps -> FrameStats and > EncodedFrameWithTimestamps -> EncodedFrameWithStats. > > Also please initialize this field. I intended to do so, but it's a little bit complex comparing to this small change. No matter, I will do it if you suggest so.
The CQ bit was checked by zijiehe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.chromium.org/2767193007/#ps40001 (title: "Resolve review comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490408385562000, "parent_rev": "f88026ebc232158b4f1f36bb76bde4a855d5b6ef", "commit_rev": "f478d76ee3acd92b6a2cf5ff2e5a0e4094c756db"}
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Send DesktopFrame::capturer_id() to client through WebrtcVideoStream Once DirectX Capturer is enabled or being experimented, we can use the capturer_id() logged in clients to analyze the failure rate. BUG=650926, 679523 ========== to ========== [Chromoting] Send DesktopFrame::capturer_id() to client through WebrtcVideoStream Once DirectX Capturer is enabled or being experimented, we can use the capturer_id() logged in clients to analyze the failure rate. BUG=650926, 679523 Review-Url: https://codereview.chromium.org/2767193007 Cr-Commit-Position: refs/heads/master@{#459625} Committed: https://chromium.googlesource.com/chromium/src/+/f478d76ee3acd92b6a2cf5ff2e5a... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/chromium/src/+/f478d76ee3acd92b6a2cf5ff2e5a...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:40001) has been created in https://codereview.chromium.org/2779483002/ by thakis@chromium.org. The reason for reverting is: Speculative, looks like this broke Webrtc_ConnectionTest.SecondCaptureFailed_0 on msan: https://build.chromium.org/p/chromium.memory.full/builders/Linux%20MSan%20Tes....
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Send DesktopFrame::capturer_id() to client through WebrtcVideoStream Once DirectX Capturer is enabled or being experimented, we can use the capturer_id() logged in clients to analyze the failure rate. BUG=650926, 679523 Review-Url: https://codereview.chromium.org/2767193007 Cr-Commit-Position: refs/heads/master@{#459625} Committed: https://chromium.googlesource.com/chromium/src/+/f478d76ee3acd92b6a2cf5ff2e5a... ========== to ========== [Chromoting] Send DesktopFrame::capturer_id() to client through WebrtcVideoStream Once DirectX Capturer is enabled or being experimented, we can use the capturer_id() logged in clients to analyze the failure rate. BUG=650926, 679523 Review-Url: https://codereview.chromium.org/2767193007 Cr-Commit-Position: refs/heads/master@{#459625} Committed: https://chromium.googlesource.com/chromium/src/+/f478d76ee3acd92b6a2cf5ff2e5a... ==========
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by zijiehe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.chromium.org/2767193007/#ps60001 (title: "crash in msan")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490575927668230, "parent_rev": "4efed9fa047ba47f6f4849a597c49a741fafbd18", "commit_rev": "805f2ca5aa0902f56885ea3c8c0a42cb80d84522"}
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Send DesktopFrame::capturer_id() to client through WebrtcVideoStream Once DirectX Capturer is enabled or being experimented, we can use the capturer_id() logged in clients to analyze the failure rate. BUG=650926, 679523 Review-Url: https://codereview.chromium.org/2767193007 Cr-Commit-Position: refs/heads/master@{#459625} Committed: https://chromium.googlesource.com/chromium/src/+/f478d76ee3acd92b6a2cf5ff2e5a... ========== to ========== [Chromoting] Send DesktopFrame::capturer_id() to client through WebrtcVideoStream Once DirectX Capturer is enabled or being experimented, we can use the capturer_id() logged in clients to analyze the failure rate. BUG=650926, 679523 Review-Url: https://codereview.chromium.org/2767193007 Cr-Original-Commit-Position: refs/heads/master@{#459625} Committed: https://chromium.googlesource.com/chromium/src/+/f478d76ee3acd92b6a2cf5ff2e5a... Review-Url: https://codereview.chromium.org/2767193007 Cr-Commit-Position: refs/heads/master@{#459684} Committed: https://chromium.googlesource.com/chromium/src/+/805f2ca5aa0902f56885ea3c8c0a... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/805f2ca5aa0902f56885ea3c8c0a... |