Bug 39446 - Video elements show no video on Windows machines that do not support accelerated compositing
Summary: Video elements show no video on Windows machines that do not support accelera...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Windows XP
: P2 Normal
Assignee: Nobody
URL: http://web.me.com/alice.liu/readerGoe...
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2010-05-20 13:59 PDT by Jer Noble
Modified: 2011-05-06 09:30 PDT (History)
6 users (show)

See Also:


Attachments
Patch (7.56 KB, patch)
2010-05-20 14:19 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (6.25 KB, patch)
2010-05-20 14:28 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (6.10 KB, patch)
2010-05-20 14:40 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (Part2) (4.38 KB, patch)
2010-05-26 14:31 PDT, Jer Noble
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (5.78 KB, patch)
2010-05-26 18:13 PDT, Jer Noble
adele: review-
Details | Formatted Diff | Diff
updated patch (7.61 KB, patch)
2010-05-26 21:24 PDT, Adele Peterson
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2010-05-20 13:59:42 PDT
Video elements show no video on Windows machines that do not support accelerated compositing
Comment 1 Jer Noble 2010-05-20 14:04:19 PDT
rdar://problem/7999794
Comment 2 Jer Noble 2010-05-20 14:19:38 PDT
Created attachment 56629 [details]
Patch
Comment 3 Jer Noble 2010-05-20 14:28:28 PDT
Created attachment 56632 [details]
Patch

Patch was against an old version of the file; updated and regenerated the patch.  Hopefully, this will let the bots run correctly now.
Comment 4 Jer Noble 2010-05-20 14:40:07 PDT
Created attachment 56634 [details]
Patch
Comment 5 Jer Noble 2010-05-20 15:45:00 PDT
Committed r59871: <http://trac.webkit.org/changeset/59871>
Comment 6 Eric Seidel (no email) 2010-05-20 15:55:02 PDT
Looks like this broke windows compile.  Sorry the win-ews bot has been down for so long, it just restarted running yesterday.
Comment 7 Jer Noble 2010-05-20 16:14:58 PDT
Committed r59873: <http://trac.webkit.org/changeset/59873>
Comment 8 Eric Carlson 2010-05-26 14:21:26 PDT
Problem not fixed by these changes.
Comment 9 Jer Noble 2010-05-26 14:24:24 PDT
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.
Comment 10 Jer Noble 2010-05-26 14:31:00 PDT
Created attachment 57138 [details]
Patch (Part2)
Comment 11 WebKit Review Bot 2010-05-26 14:34:54 PDT
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] [1]
WebCore/platform/graphics/win/QTMovieVisualContext.cpp:88:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp:991:  Mismatching spaces inside () in if  [whitespace/parens] [5]
WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp:1004:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 8 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Sam Weinig 2010-05-26 14:37:58 PDT
Comment on attachment 57138 [details]
Patch (Part2)

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 60251)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,23 @@
> +2010-05-26  Jer Noble  <jer.noble@apple.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
> +		

TABS!


> +    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 )

Extra space.

> +        options = QTMovieVisualContext::getCGImageOptions();
> +    m_visualContext = adoptRef(new QTMovieVisualContext(m_visualContextClient.get(), options));

This should use a create() method.

> +    m_visualContext->setMovie(m_movie.get());
>  }


>  QTMovieVisualContextPriv::~QTMovieVisualContextPriv()
>  {
> +	if (m_visualContext)
> +		QTVisualContextSetImageAvailableCallback(m_visualContext, 0, 0);
TABS!


Other than the nits. r=me.
Comment 13 Jer Noble 2010-05-26 18:13:10 PDT
Created attachment 57182 [details]
Patch
Comment 14 Adele Peterson 2010-05-26 21:23:49 PDT
Comment on attachment 57182 [details]
Patch

When testing with hardware acceleration turned off in D3D, this crashed several times.  Posting a new patch that I think should address the issues.
Comment 15 Adele Peterson 2010-05-26 21:24:42 PDT
Created attachment 57190 [details]
updated patch

This should really be reviewed by someone who knows this code.
Comment 16 Adele Peterson 2010-05-26 22:45:11 PDT
hmm a more accurate description in the ChangeLog would be "Patch edited by Adele Peterson and Mark Rowe"
Comment 17 Eric Carlson 2010-05-26 23:00:28 PDT
http://trac.webkit.org/changeset/60272
Comment 18 WebKit Review Bot 2011-05-05 23:23:40 PDT
Comment on attachment 57138 [details]
Patch (Part2)

Attachment 92543 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8590160

New failing tests:
http/tests/websocket/tests/url-with-nonascii-query.html
Comment 19 Eric Carlson 2011-05-05 23:25:59 PDT
(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:
> http/tests/websocket/tests/url-with-nonascii-query.html

This patch causes an unrelated test to fail a year after it was submitted?
Comment 20 Eric Seidel (no email) 2011-05-06 09:29:38 PDT
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. :(
Comment 21 Eric Seidel (no email) 2011-05-06 09:30:43 PDT
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!