Bug 177869 – Make RealtimeIncomingVideoSources and RealtimeOutgoingVideoSources port agnostic

• Component: WebRTC
• Status: RESOLVED FIXED

[Alejandro G. Castro]

We need this in the GTK port in order to use libwebrtc backend and endpoint classes.

[Alejandro G. Castro]

Created attachment 322658 [details]
Patch

[Build Bot]

Attachment 322658 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:73:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:74:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322663 [details]
Patch

[Build Bot]

Attachment 322663 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:83:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:84:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322665 [details]
Patch

[Build Bot]

Attachment 322665 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:83:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:84:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322672 [details]
Patch

[Build Bot]

Attachment 322672 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 322675 [details]
Patch

[Build Bot]

Attachment 322675 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322679 [details]
Patch

[Build Bot]

Attachment 322679 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322685 [details]
Patch

[Build Bot]

Attachment 322685 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322810 [details]
Patch

[Build Bot]

Attachment 322810 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Comment on attachment 322810 [details]
Patch

r=me provided that bots are happy.
Some comments below to improve the code since we are touching it.

Thinking about it, I guess RealtimeXXMac.h/.cpp should probably named RealtimeXXCocoa.h/.cpp

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

> Source/WebCore/platform/mediastream/RealtimeIncomingVideoSource.cpp:47
> +    auto source = RealtimeIncomingVideoSourceMac::create(WTFMove(videoTrack), WTFMove(trackId));

We can probably move RealtimeIncomingVideoSource::create implementation to RealtimeIncomingVideoSourceMac.cpp.
That would remove the FIXME and the need to include RealtimeIncomingVideoSourceMac.h here.
We duplicate source->start() but this is probably ok.

> Source/WebCore/platform/mediastream/RealtimeIncomingVideoSource.h:67
> +    void OnFrame(const webrtc::VideoFrame&) override { };

Can we remove this one since GTK is not compiling with LIBWEBRTC on currently?

> Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:49
> +    return RealtimeOutgoingVideoSourceMac::create(WTFMove(videoSource));

Ditto here for moving this to RealtimeOutgoingVideoSourceMac.cpp.

> Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:87
> +    bool GetStats(Stats*) final;

Could be inlined since it is a one liner function.

> Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:99
> +    virtual void videoSampleAvailable(MediaSample&) { };

Since videoSampleAvailable would be made virtual, maybe it is best to kill it and implement the already virtual sampleBufferUpdated directly in the port specific classes.

> Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceMac.cpp:149
> +    callOnMainThread([protectedThis = WTFMove(protectedThis), sample = WTFMove(sample), width, height, rotation] {

protectedThis = makeRef(*this)

> Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceMac.h:16
> + *    software without specific prior written permission.

Remove this third clause here and in other Mac.h/.cpp files since they are new.

> Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSourceMac.h:40
> +    ~RealtimeOutgoingVideoSourceMac() { }

Probably not needed.

[Alejandro G. Castro]

(In reply to youenn fablet from comment #17)
> Comment on attachment 322810 [details]
> Patch
> 
> r=me provided that bots are happy.

There is still some compilation issue with the mac ones I try to find.

> Some comments below to improve the code since we are touching it.
> 
> Thinking about it, I guess RealtimeXXMac.h/.cpp should probably named
> RealtimeXXCocoa.h/.cpp
> 

Right.

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=322810&action=review
> 
> > Source/WebCore/platform/mediastream/RealtimeIncomingVideoSource.cpp:47
> > +    auto source = RealtimeIncomingVideoSourceMac::create(WTFMove(videoTrack), WTFMove(trackId));
> 
> We can probably move RealtimeIncomingVideoSource::create implementation to
> RealtimeIncomingVideoSourceMac.cpp.
> That would remove the FIXME and the need to include
> RealtimeIncomingVideoSourceMac.h here.
> We duplicate source->start() but this is probably ok.
> 

Right.

> > Source/WebCore/platform/mediastream/RealtimeIncomingVideoSource.h:67
> > +    void OnFrame(const webrtc::VideoFrame&) override { };
> 
> Can we remove this one since GTK is not compiling with LIBWEBRTC on
> currently?
> 

Right, until we upload the branch.

> > Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:49
> > +    return RealtimeOutgoingVideoSourceMac::create(WTFMove(videoSource));
> 
> Ditto here for moving this to RealtimeOutgoingVideoSourceMac.cpp.
> 

Right.

> > Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:87
> > +    bool GetStats(Stats*) final;
> 
> Could be inlined since it is a one liner function.
> 

Right.

> > Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:99
> > +    virtual void videoSampleAvailable(MediaSample&) { };
> 
> Since videoSampleAvailable would be made virtual, maybe it is best to kill
> it and implement the already virtual sampleBufferUpdated directly in the
> port specific classes.
> 

Right.

> > Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceMac.cpp:149
> > +    callOnMainThread([protectedThis = WTFMove(protectedThis), sample = WTFMove(sample), width, height, rotation] {
> 
> protectedThis = makeRef(*this)
> 

Right

> > Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceMac.h:16
> > + *    software without specific prior written permission.
> 
> Remove this third clause here and in other Mac.h/.cpp files since they are
> new.
> 

Right.

> > Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSourceMac.h:40
> > +    ~RealtimeOutgoingVideoSourceMac() { }
> 
> Probably not needed.

I'll try it.

[Alejandro G. Castro]

Created attachment 322825 [details]
Patch

[Build Bot]

Attachment 322825 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322829 [details]
Patch

[Build Bot]

Attachment 322829 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 322830 [details]
Patch

[Build Bot]

Attachment 322830 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 323403 [details]
Patch

[Build Bot]

Attachment 323403 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 323405 [details]
Patch

[Build Bot]

Attachment 323405 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alejandro G. Castro]

Created attachment 323636 [details]
Patch

[Build Bot]

Attachment 323636 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:85:  is_screencast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:86:  needs_denoising is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 323636 [details]
Patch

Clearing flags on attachment: 323636

Committed r223406: <https://trac.webkit.org/changeset/223406>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35006338>