Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2323573004:
Fix crop bug in WebRtcVideoCapturerAdapter (Closed)
|
Created:
4 years, 3 months ago by magjed_chromium Modified:
4 years, 3 months ago CC:
chromium-reviews, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, jam, feature-media-reviews_chromium.org, darin-cc_chromium.org, mcasas+watch+vc_chromium.org, miu+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix crop bug in WebRtcVideoCapturerAdapter
In the current code, CPU adaptation does not work correctly. It should
scale to the adapted resolution, but it crops instead. The result looks
like zoomed in video. This bug was introduced in
https://codereview.chromium.org/2270723002/.
This CL fixes the cropping logic.
BUG=chromium:635592
Committed: https://crrev.com/aca43aa35c6bb786b83932e87b3ca8c0d8149f81
Cr-Commit-Position: refs/heads/master@{#417549}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 18 (11 generated)
Description was changed from ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes that issue. BUG=chromium:635592 ========== to ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes that issue. BUG=chromium:635592 ==========
magjed@chromium.org changed reviewers: + nisse@chromium.org, perkj@chromium.org
Description was changed from ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes that issue. BUG=chromium:635592 ========== to ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes this bug. BUG=chromium:635592 ==========
Description was changed from ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes this bug. BUG=chromium:635592 ========== to ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes the cropping logic. BUG=chromium:635592 ==========
Please take a look.
The CQ bit was checked by magjed@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...
LGTM, with or without the video_frame local variable. https://codereview.chromium.org/2323573004/diff/1/content/renderer/media/webr... File content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc (right): https://codereview.chromium.org/2323573004/diff/1/content/renderer/media/webr... content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc:157: scoped_refptr<media::VideoFrame> video_frame = It would more readable to write frame = media::VideoFrame::WrapVideoFrame(frame, ...) instead of introducing a new variable video_frame. But there are two uses of the original value of frame below (frame->timestamp() and Bind(&ReleaseOriginalFrame, frame), so perhaps that's not so easy.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
https://codereview.chromium.org/2323573004/diff/1/content/renderer/media/webr... File content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc (right): https://codereview.chromium.org/2323573004/diff/1/content/renderer/media/webr... content/renderer/media/webrtc/webrtc_video_capturer_adapter.cc:157: scoped_refptr<media::VideoFrame> video_frame = On 2016/09/09 09:06:57, nisse-chromium wrote: > It would more readable to write > > frame = media::VideoFrame::WrapVideoFrame(frame, ...) > > instead of introducing a new variable video_frame. But there are two uses of the > original value of frame below (frame->timestamp() and > Bind(&ReleaseOriginalFrame, frame), so perhaps that's not so easy. I agree it's confusing with multiple frames, but I don't dare to touch this code more than necessary for this fix :)
The CQ bit was checked by magjed@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes the cropping logic. BUG=chromium:635592 ========== to ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes the cropping logic. BUG=chromium:635592 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes the cropping logic. BUG=chromium:635592 ========== to ========== Fix crop bug in WebRtcVideoCapturerAdapter In the current code, CPU adaptation does not work correctly. It should scale to the adapted resolution, but it crops instead. The result looks like zoomed in video. This bug was introduced in https://codereview.chromium.org/2270723002/. This CL fixes the cropping logic. BUG=chromium:635592 Committed: https://crrev.com/aca43aa35c6bb786b83932e87b3ca8c0d8149f81 Cr-Commit-Position: refs/heads/master@{#417549} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/aca43aa35c6bb786b83932e87b3ca8c0d8149f81 Cr-Commit-Position: refs/heads/master@{#417549} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
