Bug 193449 - Multiple playing videos pool needs to be managed by browser
Summary: Multiple playing videos pool needs to be managed by browser
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: iPhone / iPad All
: P2 Critical
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-15 08:21 PST by Ali G
Modified: 2022-01-31 08:38 PST (History)
11 users (show)

See Also:


Attachments
Testcase (712 bytes, text/html)
2019-01-16 05:04 PST, Frédéric Wang (:fredw)
no flags Details
iPad crash log (20.28 KB, text/plain)
2019-01-29 06:16 PST, Frédéric Wang (:fredw)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ali G 2019-01-15 08:21:24 PST
1- iOS by design can only play <x> concurrent videos as decoding is done on hardware. <x> depends on the device, for iPhone 6, it is 32.
2- If a video is "paused" it still occupies the hardware decoder and is considered part of the <x> concurrent videos limit.
3- This is all fine but default behaviour of the API is not. Essentially on <x>+1 video play, the call to `play()` method fails. It should instead evict one of the existing videos and play the x+1 instead of failing on the last one. The heuristics for evict could be FIFO based or something else, does not really matter.
4- The reason the current way the API works is bad, is that it pushes all the pool management back to the app developer and given this is iOS only and high-level, it is super easy to miss the need for this and it is a pain to code a state management here given the webpage may not even have access to all the playing videos to evict them ( they could be in an iframe).

To see this in action, visit: https://s.codepen.io/aghassemi/debug/VqgzjV on a real device (not sure what the # of hardware decoders of iPhone > 6 is but try with 6 if you can).
As you scroll this page, when a video comes into view, it plays, as it exists the view it gets paused. Notice that video #33 does not play because 32 other videos have been paused.

Thanks!
-Ali
Comment 1 Radar WebKit Bug Importer 2019-01-15 15:48:27 PST
<rdar://problem/47299934>
Comment 2 Frédéric Wang (:fredw) 2019-01-16 05:04:38 PST
Created attachment 359263 [details]
Testcase

Reduced testcase that does not require AMP tags.
Comment 3 Frédéric Wang (:fredw) 2019-01-29 03:21:23 PST
This testcase is crashing in iOS 12.2 beta 1 (january 24), so I'm setting this as critical.

I tried to debug this quickly last week, but I haven't got a chance to go deeper. Also it's actually not reproducible with the simulator (it only happens with hardware) so that makes the task tricky. I see that we arrive in platformPlay() from MediaPlayerPrivateAVFoundationObjC.mm . IIUC, the failure is likely to happen when we call MediaPlayerPrivateAVFoundationObjC::createAVPlayer() to create an AVPlayer: https://developer.apple.com/documentation/avfoundation/avplayer

I'm wondering if instead of failing, we should release an existing AVPlayer as Ali suggested. Any suggestion?
Comment 4 Eric Carlson 2019-01-29 05:43:19 PST
Please attach a crash log
Comment 5 Frédéric Wang (:fredw) 2019-01-29 06:16:32 PST
Created attachment 360458 [details]
iPad crash log

This is the file generated in /Users/fred/Library/Logs/CrashReporter/MobileDevice/
Comment 6 Brent Fulgham 2019-01-29 14:02:42 PST
I don't see the security issue here. Can you clarify why this is in the Security component?
Comment 7 Frédéric Wang (:fredw) 2019-01-30 00:14:21 PST
(In reply to Brent Fulgham from comment #6)
> I don't see the security issue here. Can you clarify why this is in the
> Security component?

Sorry I had no idea what this crash was about so I thought it should be safer to move it to security before commenting on this publicly. If not please, remove it from the security component.
Comment 8 Brent Fulgham 2019-05-22 09:02:42 PDT
Moved out of Security component since it is not a security issue.
Comment 9 Frédéric Wang (:fredw) 2019-11-24 23:30:19 PST
(In reply to Frédéric Wang (:fredw) from comment #5)
> Created attachment 360458 [details]
> iPad crash log
> 
> This is the file generated in
> /Users/fred/Library/Logs/CrashReporter/MobileDevice/

Crash still happens in iOS 13.3 beta (17C5046a)