Bug 147202 – Linking WebKit2 to be able to grab media sources from a UID

• Component: WebKit API
• Status: RESOLVED FIXED

[Matthew Daiter]

Need to be able to grab TrackSourceInfo from a UID, mainly names of devices to display.

[Radar WebKit Bug Importer]

<rdar://problem/21947608>

[Matthew Daiter]

Created attachment 257292 [details]
Patch

[Darin Adler]

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()) {

[Brent Fulgham]

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.

[Matthew Daiter]

Created attachment 257355 [details]
Patch

[Darin Adler]

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>.

[Matthew Daiter]

Created attachment 257402 [details]
Patch

[Matthew Daiter]

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.

[Matthew Daiter]

Created attachment 257404 [details]
Patch

[Brent Fulgham]

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 */)"

[Brent Fulgham]

Build error after this patch:

.../openwebrtc/RealtimeMediaSourceCenterOwr.cpp:185:138: error: unused parameter 'constraints' [-Werror=unused-parameter]

[Matthew Daiter]

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.

[Matthew Daiter]

Created attachment 257405 [details]
Patch

[Brent Fulgham]

../../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*)

[Brent Fulgham]

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!

[Matthew Daiter]

Created attachment 257413 [details]
Patch

[Brent Fulgham]

Comment on attachment 257413 [details]
Patch

Looks good!

[WebKit Commit Bot]

Comment on attachment 257413 [details]
Patch

Clearing flags on attachment: 257413

Committed r187282: <http://trac.webkit.org/changeset/187282>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.