Summary: | Video freezes when attaching a local MediaStream to multiple elements | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dan <kentbriggs5> | ||||||||||
Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer, youennf | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Safari Technology Preview | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 212073 | ||||||||||||
Attachments: |
|
Description
Dan
2019-02-18 17:57:12 PST
iOS 12.2 just landed stable and this issue is still happening. My iPhone 5s that was working fine on 12.1.3 now has this issue on 12.2. Thanks Dan, I was able to repro on STP and iOS. Often the video freezes for a few seconds and then restarts again on its own. Doing play/pause usually works for the video but some other video might get frozen for some time. Adding controls and user clicking pause/play does fix the freezing. Ya good call. I tried .play() but hadn't tried pausing first. That works for me as well. Do you know if it's possible to detect this frozen state? Only thing I could think of was using a canvas with getImageData() to check if rgba values have changed. Pretty hacky to say the least :) Just wanted to mention that this problem only occurs for local streams. I was curious if it would also happen for remote streams so I made another codepen to test. Remote streams seem to work fine. Here's the pen in case anyone wants to play around with it: https://codepen.io/dbriggs/pen/KELaYQ. Did a quick check and flushing the AVSampleBufferDisplayLayer before enqueueing a sample in MediaPlayerPrivateMediaStreamAVFObjC::enqueueCorrectedVideoSample seems to do the trick. (In reply to youenn fablet from comment #7) > Did a quick check and flushing the AVSampleBufferDisplayLayer before > enqueueing a sample in > MediaPlayerPrivateMediaStreamAVFObjC::enqueueCorrectedVideoSample seems to > do the trick. We don't want to do that every time, so we'll have to figure out how to detect the bad state. Any progress on this? I've noticed two similar nasty issues recently and I'm wondering if they might be caused by the same underlying problem. 1. The video from remote streams sometimes freezes if you minimize the browser then come back a few seconds later. This is quite rare and I haven't found a reliable way to reproduce it. But the it looks the same as the original issue I described. Everything about the video track is normal (enabled, not muted, readyState is live, etc), video element is not paused, and calling .pause() then .play() resolves it. But I haven't found a way to detect it. I also logged getStats results in my test to make sure I was receiving video data. Everything looks fine, the video is just frozen on the screen. 2. Remote audio sometimes freezes. I've seen something very similar to the point above but for audio. We've had several users of our application report this issue. It shows up as one person on iOS safari not being able to hear one of the other participants. To be clear, they can hear everyone else, and everyone can hear them. They just can't hear 1 person. I managed to reproduce it only a couple of times after trying for several hours (hundreds of attempts - not highly reproducible!). Similar to the video issue above, the viewer is receiving audio data and the audio track looks fine, plus, the audio element is not paused. But audio can't be heard. Calling .pause() then .play() fixes this as well. The problem is that I can't detect it. I'm even using the web audio API to ensure we are getting sound and indeed we are. Everything seems to be working well, the user just can't hear the sound. I've considered just calling .pause() then .play() on an interval to ensure it never gets stuck in this state, but the paused state seems to last almost a second on my iPhone. Having audio cut out for a second on a regular interval just isn't an option :) Let me know if you think these might be related to the original report. If not, I'm happy to file a new bug report. These are big problems for us so I'd love to get them solved. The remote video issue might be the same. The audio bug seems different, can you file another bug? If you can reproduce it, taking a sysdiagnose might be helpful. Thanks for the quick response! Took a couple hours but was finally able to reproduce the audio issue again and capture a sysdiagnose :) Filed https://bugs.webkit.org/show_bug.cgi?id=198545. Another similar issue happened to me: - load https://webrtc.github.io/samples/src/content/peerconnection/pc1/ - click start and call to have an on-going webrtc call - load youtube.com in another tab and start a video. At that point, the capture should be suspended - go back to the previous tab, the capture should restart (and youtube be suspended). At that point, the video element with the local track was black, the video element with the remote track was ok. Interesting. Both video elements go black for me when returning from YouTube (elements are paused). If you inspect the element, is it paused? Or black for some other reason? Calling .play() on the elements fixes it for me. Created attachment 400232 [details]
Patch
Created attachment 400240 [details]
Patch
Created attachment 400249 [details]
Patch
Comment on attachment 400249 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400249&action=review > Source/WebCore/ChangeLog:5 > + <rdar://problem/49274083> I think you want rdar://63613107 > Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm:307 > + // If needed, we set the sample buffer to kCMSampleAttachmentKey_DisplayImmediately as a workaround to rdar://problem/49274083. > + // We clone the sample buffer as modifying the attachments of a sample buffer used elsewhere (encoding e.g.) may not be thread safe. Nit: this comment should be above the `if (m_renderPolicy == RenderPolicy::Immediately)` > Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:412 > + if (status) > + return nullptr; This leaks formatDescription, maybe make it a RetainPtr? > Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:418 > + if (status) > + return nullptr; Ditto. Comment on attachment 400249 [details]
Patch
r=me once the bots are happy.
Created attachment 400314 [details]
Patch for landing
Thanks for the review, I updated the patch accordingly Committed r262189: <https://trac.webkit.org/changeset/262189> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400314 [details]. |