Bug 188309

Summary: NotReadableError when calling getUserMedia
Product: WebKit Reporter: Ben <ben.browitt>
Component: WebRTCAssignee: Eric Carlson <eric.carlson>
Status: REOPENED ---    
Severity: Normal CC: bfulgham, commit-queue, eric.carlson, ews-watchlist, stephen_cobb, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Mac   
OS: macOS 10.13   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews206 for win-future
none
Patch none

Description Ben 2018-08-03 09:17:16 PDT
Calling getUserMedia in Safari Technology Preview gives an error:
NotReadableError: The I/O read operation failed.

Safari 11.1.2 works fine.

Tested with Safari Technology Preview Release 62
https://webrtc.github.io/samples/src/content/getusermedia/gum/
Comment 1 Radar WebKit Bug Importer 2018-08-03 15:32:21 PDT
<rdar://problem/42916838>
Comment 2 stephen_cobb 2018-08-03 16:03:15 PDT
I am unable to reproduce on Mojave running STP Release 62 (Safari 12.1, WebKit 14607.1.1)

I am also unable to reproduce on High Sierra Release 62 STP (Safari 12.1, WebKit 13607.1.1)

Could you please capture a sysdiagnose after reproducing this issue? Thank you.
Comment 3 Ben 2018-08-04 10:35:17 PDT
I'm using:
MacBook Pro (13-inch Late 2011)
macOS High Sierra Version 10.13.6
Safari Technology Preview Release 62 (Safari 12.1, WebKit 13607.1.1

I tested again and now I'm not getting the error so feel free to close the issue.
Comment 4 Eric Carlson 2018-08-04 11:19:42 PDT
It sounds like the camera and/or microphone wasn't available when you tested before.

Thanks for reporting the bug and for updating it so quickly Ben!
Comment 5 Ben 2018-08-04 17:29:37 PDT
I have a test case that cause the NotReadableError every time both on Safari Technology Preview 12.1 (WebKit 13607.1.1) and Safari on iOS 12 Beta 4:
https://benbro.github.io/samples/src/content/peerconnection/pc1/

Click on the Start and Call buttons. It'll call getUserMedia with just audio, add it to the peer connection and negotiate.
Click on the GUM button. This time it call getUserMedia with both audio and video and you should see the NotReadableError.
Comment 6 Eric Carlson 2018-08-06 13:24:46 PDT
Created attachment 346648 [details]
Patch
Comment 7 Brent Fulgham 2018-08-06 13:31:33 PDT
Comment on attachment 346648 [details]
Patch

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

Looks good r=me

> Source/WebKit/UIProcess/UserMediaProcessManager.cpp:151
> +    if (withAudio && !(currentExtensions & ProcessState::SandboxExtensionsGranted::Audio)) {

All of this might be slightly easier to follow if we used a helper function to check for extension types (e.g., "bool hasAudioExtension(unsigned currentExtensions) { return currentExtensions &  ProcessState::SandboxExtensionsGranted::Audio; }" and so forth).

But not needed for this bug fix.
Comment 8 EWS Watchlist 2018-08-07 10:22:24 PDT
Comment on attachment 346648 [details]
Patch

Attachment 346648 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8788544

New failing tests:
imported/blink/transitions/unprefixed-transform.html
Comment 9 EWS Watchlist 2018-08-07 10:22:36 PDT
Created attachment 346712 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 10 Eric Carlson 2018-08-07 10:26:23 PDT
(In reply to Build Bot from comment #9)
> Created attachment 346712 [details]
> Archive of layout-test-results from ews206 for win-future
> 
> The attached test failures were seen while running run-webkit-tests on the
> win-ews.
> Bot: ews206  Port: win-future  Platform:
> CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

These are unrelated.
Comment 11 Eric Carlson 2018-08-07 10:27:14 PDT
Comment on attachment 346648 [details]
Patch

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

>> Source/WebKit/UIProcess/UserMediaProcessManager.cpp:151
>> +    if (withAudio && !(currentExtensions & ProcessState::SandboxExtensionsGranted::Audio)) {
> 
> All of this might be slightly easier to follow if we used a helper function to check for extension types (e.g., "bool hasAudioExtension(unsigned currentExtensions) { return currentExtensions &  ProcessState::SandboxExtensionsGranted::Audio; }" and so forth).
> 
> But not needed for this bug fix.

Good idea, I will upload a new patch that does this.
Comment 12 Eric Carlson 2018-08-07 10:28:30 PDT
Created attachment 346713 [details]
Patch
Comment 13 Brent Fulgham 2018-08-07 11:05:18 PDT
Comment on attachment 346713 [details]
Patch

Wow -- this is a lot easier to understand. r=me
Comment 14 WebKit Commit Bot 2018-08-07 11:33:31 PDT
Comment on attachment 346713 [details]
Patch

Clearing flags on attachment: 346713

Committed r234662: <https://trac.webkit.org/changeset/234662>
Comment 15 WebKit Commit Bot 2018-08-07 11:33:33 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Ben 2018-09-04 09:15:55 PDT
I'm still getting NotReadableError on iOS 12 Beta 10 with the test
https://benbro.github.io/samples/src/content/peerconnection/pc1/

Tested with:
iOS 12.0 Beta 10 (605.1.15)
Comment 17 Ben 2018-09-04 23:13:18 PDT
Still broken on Safari 12 (AppleWebkit/605.1.15) iOS beta 10.
Comment 18 Ben 2018-09-17 15:23:30 PDT
Still broken on Safari 12 (AppleWebkit/605.1.15) iOS 12.
Comment 19 youenn fablet 2018-09-17 15:47:47 PDT
(In reply to Ben from comment #18)
> Still broken on Safari 12 (AppleWebkit/605.1.15) iOS 12.

Thanks Ben, I also was able to repro the issue.
Will need to investigate further to find the root cause.
Comment 20 Ben 2018-11-16 08:51:08 PST
Still broken on iOS 12.1
Comment 21 Ben 2019-01-31 11:49:02 PST
If you first call GUM with just video and than again with audio+video you won't get an error but the audio packets with have zero volume.
Comment 22 Ben 2019-02-06 00:07:21 PST
youenn: any update on this bug?
Comment 23 youenn fablet 2019-02-06 07:49:14 PST
(In reply to Ben from comment #22)
> youenn: any update on this bug?

Trying on STP and iOS12.2 beta, I cannot repro it anymore.
I will try on another device though.
I saw some crashes but https://bugs.webkit.org/show_bug.cgi?id=194181 should solve this.
Comment 24 Ben 2019-02-06 09:56:10 PST
Thank you for the fix and validation.

This bug is about first calling GUM with audio and than with audio+video. Did you also check the reverse? Calling GUM with only video and than with audio+video? In my test there is no error but the audio packets has zero volume.
Comment 25 youenn fablet 2019-02-06 12:40:34 PST
(In reply to Ben from comment #24)
> Thank you for the fix and validation.

If you have time for iOS 12.2 validation, that would be great.

> This bug is about first calling GUM with audio and than with audio+video.
> Did you also check the reverse? Calling GUM with only video and than with
> audio+video? In my test there is no error but the audio packets has zero
> volume.

Tried https://youennf.github.io/webrtc-tests/src/content/getusermedia/volume-after-video/. This seems to work fine on iOS.
On MacOS, I hit that issue a few times but am unable to repro it right now. Reloading the page fixes the issue.
Comment 26 Ben 2019-02-12 10:19:58 PST
I can reproduce both issues (error and volume zero) in iOS 12.1.4 so if you don't see them on iOS 12.2 beta that's great.

The fact that you hit the volume issue on MacOS few times is not reassuring.