Bug 168802 - [WebRTC] RealtimOutgoingVideoSource should not need to do image conversion
Summary: [WebRTC] RealtimOutgoingVideoSource should not need to do image conversion
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-02-23 14:41 PST by youenn fablet
Modified: 2017-03-02 23:34 PST (History)
5 users (show)

See Also:


Attachments
Patch (10.25 KB, patch)
2017-02-23 14:44 PST, youenn fablet
no flags Details | Formatted Diff | Diff
style (10.25 KB, patch)
2017-02-23 14:48 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Fixing build (22.18 KB, patch)
2017-02-23 15:28 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (22.03 KB, patch)
2017-02-24 14:13 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-02-23 14:41:31 PST
At least in the case of camera capture
Comment 1 youenn fablet 2017-02-23 14:44:39 PST
Created attachment 302582 [details]
Patch
Comment 2 youenn fablet 2017-02-23 14:48:10 PST
Created attachment 302583 [details]
style
Comment 3 youenn fablet 2017-02-23 15:28:59 PST
Created attachment 302589 [details]
Fixing build
Comment 4 Jon Lee 2017-02-23 16:26:15 PST
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?
Comment 5 youenn fablet 2017-02-24 14:13:42 PST
Created attachment 302688 [details]
Patch for landing
Comment 6 youenn fablet 2017-02-24 14:17:41 PST
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.
Comment 7 WebKit Commit Bot 2017-02-24 17:06:22 PST
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
Comment 8 WebKit Commit Bot 2017-02-27 08:31:02 PST
Comment on attachment 302688 [details]
Patch for landing

Clearing flags on attachment: 302688

Committed r213066: <http://trac.webkit.org/changeset/213066>
Comment 9 WebKit Commit Bot 2017-02-27 08:31:07 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Jon Lee 2017-03-02 23:34:35 PST
rdar://problem/30294032