Bug 170605 - [MediaStream iOS] Update muted state when interrupted
Summary: [MediaStream iOS] Update muted state when interrupted
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-07 10:43 PDT by Eric Carlson
Modified: 2017-04-09 18:56 PDT (History)
4 users (show)

See Also:


Attachments
Proposed patch. (4.59 KB, patch)
2017-04-07 11:03 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Updated patch. (4.14 KB, patch)
2017-04-08 11:09 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing. (5.00 KB, patch)
2017-04-08 13:02 PDT, Eric Carlson
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch for landing. (4.00 KB, patch)
2017-04-08 13:08 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 2017-04-07 10:43:42 PDT
Update muted state when interrupted
Comment 1 Radar WebKit Bug Importer 2017-04-07 10:44:27 PDT
<rdar://problem/31503896>
Comment 2 Eric Carlson 2017-04-07 11:03:07 PDT
Created attachment 306516 [details]
Proposed patch.
Comment 3 Build Bot 2017-04-07 11:04:29 PDT
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.
Comment 4 Eric Carlson 2017-04-08 11:09:34 PDT
Created attachment 306572 [details]
Updated patch.
Comment 5 youenn fablet 2017-04-08 11:36:02 PDT
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?
Comment 6 Eric Carlson 2017-04-08 13:02:05 PDT
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.
Comment 7 Eric Carlson 2017-04-08 13:02:56 PDT
Created attachment 306577 [details]
Patch for landing.
Comment 8 WebKit Commit Bot 2017-04-08 13:04:03 PDT
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
Comment 9 Eric Carlson 2017-04-08 13:08:39 PDT
Created attachment 306579 [details]
Patch for landing.
Comment 10 WebKit Commit Bot 2017-04-08 13:35:48 PDT
Comment on attachment 306579 [details]
Patch for landing.

Clearing flags on attachment: 306579

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