Bug 170605 – [MediaStream iOS] Update muted state when interrupted

• Status: ASSIGNED
• Component: Media

[Eric Carlson]

Update muted state when interrupted

[Radar WebKit Bug Importer]

<rdar://problem/31503896>

[Eric Carlson]

Created attachment 306516 [details]
Proposed patch.

[Build Bot]

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


ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:132:  The parameter name "options" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 5 files


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

[Eric Carlson]

Created attachment 306572 [details]
Updated patch.

[youenn fablet]

Comment on attachment 306572 [details]
Updated patch.

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

> Source/WebCore/ChangeLog:19
> +        call notifyMutedObservers because the session has already stopped running.

Is there a case where captureSessionIsRunningDidChange will be called for the session to start/restart?

> Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:278
> +        if (m_muted != m_isRunning) {

Shouldn't it be "if (m_muted == m_isRunning)" since we are doing "m_muted = !m_isRunning;" inside the if statement.

> Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:279
> +            // Update muted state without generating a call to StartProducingData/StopProducingData.

Not sure this comment helps. Maybe the ChangeLog is good enough?

[Eric Carlson]

Comment on attachment 306572 [details]
Updated patch.

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

>> Source/WebCore/ChangeLog:19
>> +        call notifyMutedObservers because the session has already stopped running.
> 
> Is there a case where captureSessionIsRunningDidChange will be called for the session to start/restart?

No, it is only called to notify.

>> Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:278
>> +        if (m_muted != m_isRunning) {
> 
> Shouldn't it be "if (m_muted == m_isRunning)" since we are doing "m_muted = !m_isRunning;" inside the if statement.

Oops!

>> Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:279
>> +            // Update muted state without generating a call to StartProducingData/StopProducingData.
> 
> Not sure this comment helps. Maybe the ChangeLog is good enough?

Removed.

[Eric Carlson]

Created attachment 306577 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 306577 [details]
Patch for landing.

Rejecting attachment 306577 [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', 'apply-attachment', '--no-update', '--non-interactive', 306577, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
rm/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm.rej
patching file Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
patching file Source/WebCore/platform/mediastream/RealtimeMediaSource.h
patching file Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.h
patching file Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

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

[Eric Carlson]

Created attachment 306579 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 306579 [details]
Patch for landing.

Clearing flags on attachment: 306579

Committed r215147: <http://trac.webkit.org/changeset/215147>