Bug 58339

Summary: Apple movie trailers play only audio
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Severity: Normal CC: jer.noble
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: Other   
Description Flags
Proposed patch. simon.fraser: review+

Description Eric Carlson 2011-04-12 10:06:55 PDT
Movie trailers at http://trailers.apple.com play only audio on OS X 10.7 unless the time is changed manually by dragging the timeline thumb.
Comment 1 Eric Carlson 2011-04-12 10:07:19 PDT
Comment 2 Eric Carlson 2011-04-12 10:13:58 PDT
This happens because HTMLVideoElement::setDisplayMode assumes that a frame of video will be available immediately after calling a media engine's prepareForRendering function. This is not necessarily the case with AVFoundation so HTMLVideoElement continues to display the @poster, which is a 1x1 white pixel.

The solution is to add a mechanism for a media engine to let HTMLMediaElement know when the first frame is ready to render.
Comment 3 Eric Carlson 2011-04-12 15:47:12 PDT
Created attachment 89286 [details]
Proposed patch.
Comment 4 Jer Noble 2011-04-12 17:03:22 PDT
Comment on attachment 89286 [details]
Proposed patch.

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

> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:494
> +    if (!m_haveReportedFirstVideoFrame && m_cachedHasVideo && hasAvailableVideoFrame()) {
> +        m_haveReportedFirstVideoFrame = true;
> +        m_player->firstVideoFrameAvailable();
> +    }
> +

Is -[AVPlayerLayer isReadyForDisplay] KVO-compliant?  Because it seems a shame we have to wait for some other notification before checking to see if there's a frame ready to be rendered.
Apparently it is observable.  You could add a new observer so that we run through updateStates() as soon as a frame is ready.  But that's just an optimization and not strictly necessary.

Looks good to me.
Comment 5 Simon Fraser (smfr) 2011-04-12 17:04:33 PDT
Comment on attachment 89286 [details]
Proposed patch.

r=me with a comment in email
Comment 6 Eric Carlson 2011-04-12 17:04:56 PDT