Bug 149233 - [Mac MediaStream] Cleanup capture source classes
Summary: [Mac MediaStream] Cleanup capture source classes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-09-16 15:03 PDT by Eric Carlson
Modified: 2015-09-17 08:39 PDT (History)
2 users (show)

See Also:


Attachments
Proposed patch. (37.40 KB, patch)
2015-09-16 15:43 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch (28.32 KB, patch)
2015-09-16 16:07 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch for landing (38.51 KB, patch)
2015-09-17 07:47 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2015-09-16 15:03:40 PDT
Clean up classes, prepare for adding WebAudio support.
Comment 1 Eric Carlson 2015-09-16 15:43:13 PDT
Created attachment 261332 [details]
Proposed patch.
Comment 2 WebKit Commit Bot 2015-09-16 15:45:37 PDT
Attachment 261332 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/mac/AVAudioCaptureSource.mm:39:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVAudioCaptureSource.mm:40:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:199:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:219:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:220:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.h:75:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVAudioCaptureSource.h:32:  wtf includes should be <wtf/file.h> instead of "wtf/file.h".  [build/include] [4]
Total errors found: 8 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Eric Carlson 2015-09-16 16:07:55 PDT
Created attachment 261335 [details]
Updated patch
Comment 4 WebKit Commit Bot 2015-09-16 16:11:10 PDT
Attachment 261335 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/mac/AVAudioCaptureSource.mm:38:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVAudioCaptureSource.mm:39:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVAudioCaptureSource.mm:40:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:199:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.h:75:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 5 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Jer Noble 2015-09-16 16:21:36 PDT
Comment on attachment 261335 [details]
Updated patch

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

> Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.h:59
> +    void startProducingData() override;
> +    void stopProducingData() override;
> +

Do these need to be public?

> Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:211
> +void AVMediaCaptureSource::scheduleDeferredTask(std::function<void ()> function)
> +{
> +    ASSERT(function);
> +
> +    auto weakThis = createWeakPtr();
> +    dispatch_async(dispatch_get_main_queue(), [weakThis, function] {
> +        if (!weakThis)
> +            return;
> +
> +        function();
> +    });
> +}
> +

Will this break on WK1-iOS? (where the main thread is not necessarily the web thread)  If so, we could use GenericTaskQueue<Timer> here.
Comment 6 Eric Carlson 2015-09-17 07:47:06 PDT
Created attachment 261392 [details]
Updated patch for landing
Comment 7 WebKit Commit Bot 2015-09-17 07:48:31 PDT
Attachment 261392 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:199:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:230:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:231:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.h:77:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Eric Carlson 2015-09-17 08:34:55 PDT
Committed r189913: https://trac.webkit.org/r189913
Comment 9 Radar WebKit Bug Importer 2015-09-17 08:35:49 PDT
<rdar://problem/22739163>
Comment 10 Eric Carlson 2015-09-17 08:38:54 PDT
Comment on attachment 261335 [details]
Updated patch

Clearing flags on attachment: 261335