Video elements show no video on Windows machines that do not support accelerated compositing
Created attachment 56629 [details]
Created attachment 56632 [details]
Patch was against an old version of the file; updated and regenerated the patch. Hopefully, this will let the bots run correctly now.
Created attachment 56634 [details]
Committed r59871: <http://trac.webkit.org/changeset/59871>
Looks like this broke windows compile. Sorry the win-ews bot has been down for so long, it just restarted running yesterday.
Committed r59873: <http://trac.webkit.org/changeset/59873>
Problem not fixed by these changes.
We are not creating the visual context at the right time, and in addition, we are creating a CAImageQueue-based visual context even when the MediaPlayer is non-accelerated.
Created attachment 57138 [details]
Attachment 57138 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/win/QTMovieVisualContext.cpp:87: Tab found; better to use spaces [whitespace/tab] 
WebCore/platform/graphics/win/QTMovieVisualContext.cpp:88: Tab found; better to use spaces [whitespace/tab] 
WebCore/ChangeLog:5: Line contains tab character. [whitespace/tab] 
WebCore/ChangeLog:6: Line contains tab character. [whitespace/tab] 
WebCore/ChangeLog:7: Line contains tab character. [whitespace/tab] 
WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] 
WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp:991: Mismatching spaces inside () in if [whitespace/parens] 
WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp:1004: Tab found; better to use spaces [whitespace/tab] 
Total errors found: 8 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 57138 [details]
> Index: WebCore/ChangeLog
> --- WebCore/ChangeLog (revision 60251)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,23 @@
> +2010-05-26 Jer Noble <email@example.com>
> + Reviewed by NOBODY (OOPS!).
> + Video elements show no video on Windows machines that do not support accelerated compositing
> + https://bugs.webkit.org/show_bug.cgi?id=39446
> + rdar://problem/7999794
> + CFDictionaryRef options = 0;
> + // If CAImageQueue prerequisites are not satisfied, pass in visual context pixelbuffer
> + // options which will instruct the visual context to generate CGImage compatible
> + // pixel buffers (i.e. RGBA).
> + if (!requiredDllsAvailable() || preferredMode != MediaRenderingMovieLayer )
> + options = QTMovieVisualContext::getCGImageOptions();
> + m_visualContext = adoptRef(new QTMovieVisualContext(m_visualContextClient.get(), options));
This should use a create() method.
> + m_visualContext->setMovie(m_movie.get());
> + if (m_visualContext)
> + QTVisualContextSetImageAvailableCallback(m_visualContext, 0, 0);
Other than the nits. r=me.
Created attachment 57182 [details]
Comment on attachment 57182 [details]
When testing with hardware acceleration turned off in D3D, this crashed several times. Posting a new patch that I think should address the issues.
Created attachment 57190 [details]
This should really be reviewed by someone who knows this code.
hmm a more accurate description in the ChangeLog would be "Patch edited by Adele Peterson and Mark Rowe"
Comment on attachment 57138 [details]
Attachment 92543 [details] did not pass chromium-ews (chromium-xvfb):
New failing tests:
(In reply to comment #18)
> (From update of attachment 57138 [details])
> Attachment 92543 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/8590160
> New failing tests:
This patch causes an unrelated test to fail a year after it was submitted?
I suspect the patch flaked twice in a row. We have flaky test detection on all of the bots, but that just means they run the test multiple times... if it flakes multiple times in a row, there is little we can do. :(
Actually, this was a differnet bug. the cr-linux-ews was briefly posting results using the *bug* number when it meant to use the *patch* number. This can be completely ignored. Sorry for the spam and confusion!