[Cocoa] Don't instantiate AVPlayer-based audio decoders or renderers if an element is initially muted.
Created attachment 408296 [details] Patch
Comment on attachment 408296 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408296&action=review > Source/WebCore/ChangeLog:14 > + audio decoding and rendering until the first time the AVPlayer is unmuted. ThisThe means the first un-mute may s/ThisThe/This/ > Source/WebCore/PAL/pal/spi/cocoa/AVFoundationSPI.h:77 > +@property (nonatomic, getter=_suppressesAudioRendering, setter=_setSuppressesAudioRendering:) BOOL suppressesAudioRendering Miss a ";"?
Created attachment 408366 [details] Patch
Comment on attachment 408366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408366&action=review r=me once the bots are happy > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:946 > + m_avPlayer.get().suppressesAudioRendering = YES; We should always know if an element is muted or not with the above changes, and setMuted() will always be called if the muted state changes, so how about something like `m_avPlayer.get().suppressesAudioRendering = m_muted`? > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1334 > + if (!m_muted) > + m_avPlayer.get().suppressesAudioRendering = NO; setMuted(true) may be called before playback starts, so something like `m_avPlayer.get().suppressesAudioRendering = m_muted` might be better.
Comment on attachment 408366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408366&action=review > Source/WebCore/PAL/pal/spi/cocoa/AVFoundationSPI.h:77 > +@property (nonatomic, getter=_suppressesAudioRendering, setter=_setSuppressesAudioRendering:) BOOL suppressesAudioRendering Looks like you are missing a semi-colon
Comment on attachment 408366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408366&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:946 >> + m_avPlayer.get().suppressesAudioRendering = YES; > > We should always know if an element is muted or not with the above changes, and setMuted() will always be called if the muted state changes, so how about something like `m_avPlayer.get().suppressesAudioRendering = m_muted`? If you look deeper in the context for this change, this is already inside a `if (m_muted)` block. And three lines above m_muted is set to `false`, so we literally have no choice but to set this unconditionally to YES on this line. >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1334 >> + m_avPlayer.get().suppressesAudioRendering = NO; > > setMuted(true) may be called before playback starts, so something like `m_avPlayer.get().suppressesAudioRendering = m_muted` might be better. That would cause suppression to kick in if the video was muted during playback. The existing logic definitely has edge cases where suppression won't kick in (I.e., if you do `video.src = foo; video.addEventListener('loadedmetadata', event => { video.muted = true; });`), but those edge cases feel less common, and we can always expand this behavior.
Created attachment 408375 [details] Patch for Landing
Committed r266844: <https://trac.webkit.org/changeset/266844> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408375 [details].
<rdar://problem/68654077>
I suspect this has caused <rdar://problem/68743466>.