Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2700113003:
Add RTCVideoFrame init function from CVPixelBufferRef (Closed)
|
Created:
3 years, 10 months ago by magjed_webrtc Modified:
3 years, 10 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd RTCVideoFrame init function from CVPixelBufferRef
Adds a public init function in RTCVideoFrame that makes it possible to
create a frame from a CVPixelBufferRef.
BUG=webrtc:7177
NOTRY=True
Review-Url: https://codereview.webrtc.org/2700113003
Cr-Commit-Position: refs/heads/master@{#16746}
Committed: https://chromium.googlesource.com/external/webrtc/+/c3c46246a92761334b372e833c7a0ee153171d6b
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase, and add '(instancetype)new NS_UNAVAILABLE'. #
Total comments: 2
Depends on Patchset: Messages
Total messages: 22 (16 generated)
The CQ bit was checked by magjed@webrtc.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.webrtc.org/...
Description was changed from ========== Add RTCVideoFrame init function from CVPixelBufferRef Adds a public init function in RTCVideoFrame that makes it possible to create a frame from a CVPixelBufferRef. BUG=webrtc:7177 ========== to ========== Add RTCVideoFrame init function from CVPixelBufferRef Adds a public init function in RTCVideoFrame that makes it possible to create a frame from a CVPixelBufferRef. BUG=webrtc:7177 ==========
magjed@webrtc.org changed reviewers: + denicija@webrtc.org
Daniela - please take a look.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM https://codereview.webrtc.org/2700113003/diff/1/webrtc/sdk/objc/Framework/Hea... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h (right): https://codereview.webrtc.org/2700113003/diff/1/webrtc/sdk/objc/Framework/Hea... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h:44: - (instancetype)init NS_UNAVAILABLE; Super nit: I know that this is not part of the CL, but we should also mark + (instancetype) new NS_UNAVAILABLE;
The CQ bit was checked by magjed@webrtc.org to run a CQ dry run
https://codereview.webrtc.org/2700113003/diff/1/webrtc/sdk/objc/Framework/Hea... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h (right): https://codereview.webrtc.org/2700113003/diff/1/webrtc/sdk/objc/Framework/Hea... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h:44: - (instancetype)init NS_UNAVAILABLE; On 2017/02/20 12:10:31, denicija-webrtc wrote: > Super nit: I know that this is not part of the CL, but we should also mark > + (instancetype) new NS_UNAVAILABLE; Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_clang_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_compile_mips_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_compile_x64_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_compile_x86_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_compile_x86_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_more_configs on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
Description was changed from ========== Add RTCVideoFrame init function from CVPixelBufferRef Adds a public init function in RTCVideoFrame that makes it possible to create a frame from a CVPixelBufferRef. BUG=webrtc:7177 ========== to ========== Add RTCVideoFrame init function from CVPixelBufferRef Adds a public init function in RTCVideoFrame that makes it possible to create a frame from a CVPixelBufferRef. BUG=webrtc:7177 NOTRY=True ==========
The CQ bit was checked by magjed@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from denicija@webrtc.org Link to the patchset: https://codereview.webrtc.org/2700113003/#ps20001 (title: "Rebase, and add '(instancetype)new NS_UNAVAILABLE'.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487683569854110,
"parent_rev": "2a8135a1741761bd6de52163c0dc35f6eff7c8eb", "commit_rev":
"c3c46246a92761334b372e833c7a0ee153171d6b"}
Message was sent while issue was closed.
Description was changed from ========== Add RTCVideoFrame init function from CVPixelBufferRef Adds a public init function in RTCVideoFrame that makes it possible to create a frame from a CVPixelBufferRef. BUG=webrtc:7177 NOTRY=True ========== to ========== Add RTCVideoFrame init function from CVPixelBufferRef Adds a public init function in RTCVideoFrame that makes it possible to create a frame from a CVPixelBufferRef. BUG=webrtc:7177 NOTRY=True Review-Url: https://codereview.webrtc.org/2700113003 Cr-Commit-Position: refs/heads/master@{#16746} Committed: https://chromium.googlesource.com/external/webrtc/+/c3c46246a92761334b372e833... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/webrtc/+/c3c46246a92761334b372e833...
Message was sent while issue was closed.
tkchin@webrtc.org changed reviewers: + tkchin@webrtc.org
Message was sent while issue was closed.
https://codereview.webrtc.org/2700113003/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h (right): https://codereview.webrtc.org/2700113003/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h:53: - (instancetype)new NS_UNAVAILABLE; Did you mean + ? Personally I don't think we need to disallow this everywhere. Intent should be clear enough from disallowing init. Otherwise we always need to do this in every file that has nontrivial ctor in addition to init. https://codereview.webrtc.org/2700113003/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/RTCVideoFrame.h:78: - (RTCVideoFrame *)newI420VideoFrame; there shouldn't be anything prefixed with "new" in ObjC land i420VideoFrameCopy or something |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
