Bug 168802 – [WebRTC] RealtimOutgoingVideoSource should not need to do image conversion

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

At least in the case of camera capture

[youenn fablet]

Created attachment 302582 [details]
Patch

[youenn fablet]

Created attachment 302583 [details]
style

[youenn fablet]

Created attachment 302589 [details]
Fixing build

[Jon Lee]

Comment on attachment 302589 [details]
Fixing build

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

> Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSource.cpp:34
> +#include "webrtc/common_video/include/corevideo_frame_buffer.h"

not <...>?

> Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSource.cpp:79
> +        sink->OnFrame(frame);

definitely no chance that sink==nullptr?

> Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSource.h:77
> +    std::unique_ptr<webrtc::VideoFrame> m_blackFrame;

This is never used?

[youenn fablet]

Created attachment 302688 [details]
Patch for landing

[youenn fablet]

Thanks for the review.

(In reply to comment #4)
> Comment on attachment 302589 [details]
> Fixing build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=302589&action=review
> 
> > Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSource.cpp:34
> > +#include "webrtc/common_video/include/corevideo_frame_buffer.h"
> 
> not <...>?

Fixed.

> > Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSource.cpp:79
> > +        sink->OnFrame(frame);
> 
> definitely no chance that sink==nullptr?

No.
If this was pure WebCore code, the addObserver/removeObserver would take references and m_sinks would be a Vector of std::reference_wrapper.
We could still do the latter but the removeObserver would contain more lines of code.
Maybe we should specialise Vector when containing sd::reference_wrapper though.

> > Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSource.h:77
> > +    std::unique_ptr<webrtc::VideoFrame> m_blackFrame;
> 
> This is never used?

I initially thought of having just one m_blackFrame but the buffer pool is probably good enough for now. Removed this declaration.

[WebKit Commit Bot]

Comment on attachment 302688 [details]
Patch for landing

Rejecting attachment 302688 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 302688, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/git.webkit.org/WebKit
   51cb4f6..7abdaa1  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 212976 = 51cb4f6247106b06c3049ec9e928b2855d3e0903
r212977 = 7abdaa1c8b4381865e22187a3cd9dcfbde6c2cc8
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/3187950

[WebKit Commit Bot]

Comment on attachment 302688 [details]
Patch for landing

Clearing flags on attachment: 302688

Committed r213066: <http://trac.webkit.org/changeset/213066>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jon Lee]

rdar://problem/30294032