Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2994253002:
ObjC: Include additional files in umbrella header. (Closed)
|
Created:
3 years, 4 months ago by andersc Modified:
3 years, 4 months ago Reviewers:
kthelgason CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionObjC: Include additional files in umbrella header.
RTCAudioSession and RTCAudioSessionConfiguration allow users to handle
audio manually and is used by the AppRTCMobile example.
RTCVideoFrameBuffer exposes a protocol that users can implement to
create their own frame buffer formats, as long as they can be converted
into i420.
RTCVideoCapturer and RTCVideoViewShading are imported by other headers
already included by the umbrella header, so they were always accessible
to users. Added them to the umbrella header to make it explicit.
BUG=webrtc:7351, webrtc:8027
Review-Url: https://codereview.webrtc.org/2994253002
Cr-Commit-Position: refs/heads/master@{#19379}
Committed: https://chromium.googlesource.com/external/webrtc/+/f9f448b32dacc4c0ecefb738d557f41b0deeb42d
Patch Set 1 #Patch Set 2 : Rebase #
Total comments: 2
Patch Set 3 : Only include RTCAudioSession on iOS. #
Messages
Total messages: 32 (26 generated)
andersc@webrtc.org changed reviewers: + kthelgason@webrtc.org
Moved some more files into the umbrella header. Realized that one of the reasons the RTCVideoFrameBuffer protocol was created was because users might want to specify their own frame buffer implementations (see https://bugs.chromium.org/p/webrtc/issues/detail?id=7684 ), so it should also be public.
The CQ bit was checked by andersc@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/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios_api_framework on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_api_framework/build...)
The CQ bit was checked by andersc@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/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
looks great! Only comment is that the audio session headers should probably be iOS-only as they rely on AVAudioSession. https://codereview.webrtc.org/2994253002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/WebRTC.h (right): https://codereview.webrtc.org/2994253002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/WebRTC.h:13: #import <WebRTC/RTCAudioSessionConfiguration.h> I think these should be included only on iPhone, right?
Ah, I didn't realize AVAudioSession was not available on Mac. Fixed. https://codereview.webrtc.org/2994253002/diff/20001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Headers/WebRTC/WebRTC.h (right): https://codereview.webrtc.org/2994253002/diff/20001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Headers/WebRTC/WebRTC.h:13: #import <WebRTC/RTCAudioSessionConfiguration.h> On 2017/08/16 08:06:52, kthelgason wrote: > I think these should be included only on iPhone, right? Done.
The CQ bit was checked by kthelgason@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/...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_arm64_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_arm64_rel/builds/...)
The CQ bit was checked by kthelgason@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/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios_api_framework on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_api_framework/build...)
The CQ bit was checked by andersc@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/...
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 andersc@webrtc.org
The CQ bit was unchecked by andersc@webrtc.org
The CQ bit was checked by andersc@webrtc.org
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": 40001, "attempt_start_ts": 1502962167984260,
"parent_rev": "4dee3444936f27c08e736c58c36fa37802298a6f", "commit_rev":
"f9f448b32dacc4c0ecefb738d557f41b0deeb42d"}
Message was sent while issue was closed.
Description was changed from ========== ObjC: Include additional files in umbrella header. RTCAudioSession and RTCAudioSessionConfiguration allow users to handle audio manually and is used by the AppRTCMobile example. RTCVideoFrameBuffer exposes a protocol that users can implement to create their own frame buffer formats, as long as they can be converted into i420. RTCVideoCapturer and RTCVideoViewShading are imported by other headers already included by the umbrella header, so they were always accessible to users. Added them to the umbrella header to make it explicit. BUG=webrtc:7351, webrtc:8027 ========== to ========== ObjC: Include additional files in umbrella header. RTCAudioSession and RTCAudioSessionConfiguration allow users to handle audio manually and is used by the AppRTCMobile example. RTCVideoFrameBuffer exposes a protocol that users can implement to create their own frame buffer formats, as long as they can be converted into i420. RTCVideoCapturer and RTCVideoViewShading are imported by other headers already included by the umbrella header, so they were always accessible to users. Added them to the umbrella header to make it explicit. BUG=webrtc:7351, webrtc:8027 Review-Url: https://codereview.webrtc.org/2994253002 Cr-Commit-Position: refs/heads/master@{#19379} Committed: https://chromium.googlesource.com/external/webrtc/+/f9f448b32dacc4c0ecefb738d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/webrtc/+/f9f448b32dacc4c0ecefb738d... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
