Bug 147202

Summary: Linking WebKit2 to be able to grab media sources from a UID
Product: WebKit Reporter: Matthew Daiter <mdaiter>
Component: WebKit APIAssignee: Matthew Daiter <mdaiter>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, eric.carlson, jonlee, mdaiter, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 147259    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Matthew Daiter 2015-07-22 13:34:49 PDT
Need to be able to grab TrackSourceInfo from a UID, mainly names of devices to display.
Comment 1 Radar WebKit Bug Importer 2015-07-22 13:36:03 PDT
<rdar://problem/21947608>
Comment 2 Matthew Daiter 2015-07-22 13:38:21 PDT
Created attachment 257292 [details]
Patch
Comment 3 Darin Adler 2015-07-22 14:36:50 PDT
Comment on attachment 257292 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257292&action=review

> Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:416
> +RefPtr<RealtimeMediaSource> AVCaptureDeviceManager::sourceWithUID(const String &deviceUID, RealtimeMediaSource::Type type, MediaConstraints* constraints)

Should be const String& rather than const String &.

> Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:188
> +    for (auto iter = m_sourceMap.begin(); iter != m_sourceMap.end(); ++iter) {
> +        RefPtr<RealtimeMediaSource> source = iter->value;

Should be modern for loop:

    for (auto& source : m_sourceMap.values()) {

> Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:222
> +    MockSourceMap& map = mockSourceMap();
> +    MockSourceMap::iterator end = map.end();
> +    
> +    for (MockSourceMap::iterator it = map.begin(); it != end; ++it) {
> +        MockSource* source = it->value.get();
> +

Should be a modern for loop:

    for (auto& source : mockSourceMap()) {
Comment 4 Brent Fulgham 2015-07-22 20:20:40 PDT
Comment on attachment 257292 [details]
Patch

Please upload a patch against the current ToT; this one doesn't build! Also, please fix darin's comments.
Comment 5 Matthew Daiter 2015-07-23 10:08:27 PDT
Created attachment 257355 [details]
Patch
Comment 6 Darin Adler 2015-07-23 14:19:51 PDT
Comment on attachment 257355 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257355&action=review

> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:144
> +    return TrackSourceInfo::create(mediaSource->id(), mediaSource->type() == RealtimeMediaSource::Type::Video ? TrackSourceInfo::SourceKind::Video : TrackSourceInfo::SourceKind::Audio , mediaSource->name());

Stray space here after Audio.

Might be nice to have a function to convert a RealtimeMediaSource::Type into TrackSourceInfo::SourceKind rather than writing out the ?: expression like this.

> Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.h:61
> +    const TrackSourceInfo& sourceWithUID(const String&, RealtimeMediaSource::Type, MediaConstraints*) override;

Return type needs to be RefPtr<TrackSourceInfo>.
Comment 7 Matthew Daiter 2015-07-23 16:08:26 PDT
Created attachment 257402 [details]
Patch
Comment 8 Matthew Daiter 2015-07-23 16:09:16 PDT
Comment on attachment 257355 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257355&action=review

>> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:144
>> +    return TrackSourceInfo::create(mediaSource->id(), mediaSource->type() == RealtimeMediaSource::Type::Video ? TrackSourceInfo::SourceKind::Video : TrackSourceInfo::SourceKind::Audio , mediaSource->name());
> 
> Stray space here after Audio.
> 
> Might be nice to have a function to convert a RealtimeMediaSource::Type into TrackSourceInfo::SourceKind rather than writing out the ?: expression like this.

Fixed.

>> Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.h:61
>> +    const TrackSourceInfo& sourceWithUID(const String&, RealtimeMediaSource::Type, MediaConstraints*) override;
> 
> Return type needs to be RefPtr<TrackSourceInfo>.

Fixed.
Comment 9 Matthew Daiter 2015-07-23 16:27:50 PDT
Created attachment 257404 [details]
Patch
Comment 10 Brent Fulgham 2015-07-23 16:34:25 PDT
Comment on attachment 257404 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257404&action=review

Please fix the build failure this patch introduces (see my comment).

> Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:185
> +RefPtr<TrackSourceInfo> RealtimeMediaSourceCenterOwr::sourceWithUID(const String& UID, RealtimeMediaSource::Type type, MediaConstraints* constraints)

The "constraints" parameter is causing build failures on other ports. Comment it out until you need to use it:

", MediaConstraints*)" 

or

", MediaConstraints* /* constraints */)"
Comment 11 Brent Fulgham 2015-07-23 16:35:09 PDT
Build error after this patch:

.../openwebrtc/RealtimeMediaSourceCenterOwr.cpp:185:138: error: unused parameter 'constraints' [-Werror=unused-parameter]
Comment 12 Matthew Daiter 2015-07-23 16:45:17 PDT
Comment on attachment 257404 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257404&action=review

>> Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:185
>> +RefPtr<TrackSourceInfo> RealtimeMediaSourceCenterOwr::sourceWithUID(const String& UID, RealtimeMediaSource::Type type, MediaConstraints* constraints)
> 
> The "constraints" parameter is causing build failures on other ports. Comment it out until you need to use it:
> 
> ", MediaConstraints*)" 
> 
> or
> 
> ", MediaConstraints* /* constraints */)"

Fixed.
Comment 13 Matthew Daiter 2015-07-23 16:45:33 PDT
Created attachment 257405 [details]
Patch
Comment 14 Brent Fulgham 2015-07-23 16:56:38 PDT
../../Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:185:114: error: unused parameter 'type' [-Werror=unused-parameter]
 RefPtr<TrackSourceInfo> RealtimeMediaSourceCenterOwr::sourceWithUID(const String& UID, RealtimeMediaSource::Type type, MediaConstraints*)
Comment 15 Brent Fulgham 2015-07-23 16:57:47 PDT
Comment on attachment 257405 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257405&action=review

Another build failure... I think this may be the last one, though.

> Source/WebCore/platform/mediastream/openwebrtc/RealtimeMediaSourceCenterOwr.cpp:185
> +RefPtr<TrackSourceInfo> RealtimeMediaSourceCenterOwr::sourceWithUID(const String& UID, RealtimeMediaSource::Type type, MediaConstraints*)

'type' isn't used, so it's causing a build failure. Please correct!
Comment 16 Matthew Daiter 2015-07-23 17:40:26 PDT
Created attachment 257413 [details]
Patch
Comment 17 Brent Fulgham 2015-07-23 18:29:08 PDT
Comment on attachment 257413 [details]
Patch

Looks good!
Comment 18 WebKit Commit Bot 2015-07-23 19:24:59 PDT
Comment on attachment 257413 [details]
Patch

Clearing flags on attachment: 257413

Committed r187282: <http://trac.webkit.org/changeset/187282>
Comment 19 WebKit Commit Bot 2015-07-23 19:25:03 PDT
All reviewed patches have been landed.  Closing bug.