Bug 30925 – [GTK] Black artifacts in youtube.com/html5

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Xan Lopez]

Using r50298, cairo 1.8.0, X server 1.5.3 with intel drivers 2.5.0, I get black artifacts playing the youtube demo version using HTML5 (see URL in the bug). Screenshot coming.

[Xan Lopez]

Created attachment 42136 [details]
screeshot

[Benjamin Otte]

Created attachment 42157 [details]
patch, step 1

[Benjamin Otte]

Created attachment 42158 [details]
fix

These two patches get rid of the improper alignment as exhibited in the bug.

According to Simon Fraser the right thing to do in MediaPlayerPrivate::paint() is to make the video fill the given rectangle completely and not try to keep any aspect ratio.

FWIW, at least the Qt and the win port have the same issue. I'm not sure if we should file new bugs for that or use this one?

[Jan Alonzo]

Comment on attachment 42157 [details]
patch, step 1

> +        Using cairo_paint() is faster, does not produce artefacts and is less
> +        code.

Typo: artefacts -> artifacts.

Patch looks fine. But it doesn't apply cleanly anymore. Can you please update so the commit bot can commit it?

r- because patch needs rebasing.

[Jan Alonzo]

Comment on attachment 42158 [details]
fix

LGTM. Can you please update this one as well for cq+? Thanks.

[Benjamin Otte]

Created attachment 42328 [details]
fix

Turns out using cairo_paint() is a bad idea for the gradients you get in the corner with bilinear filtering, which are bad when upscaling the image. So I removed that part and used the old cairo_fill() logic and set CAIRO_EXTEND_PAD on the source.
Other than that, the patch is unchanged.

[Benjamin Otte]

Created attachment 42329 [details]
fix

This time with only one ChangeLog entry...

[Jan Alonzo]

Comment on attachment 42329 [details]
fix

> [phone]  Benjamin Otte  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Paint the video to the given size.
> +
> +        It's the job of the callers to keep track of aspect ratio.
> +        RenderVideo.cpp does it for the <video> element.

r=me. ChangeLog is missing bug link and title though, so someone would need to fix it before landing.

[Benjamin Otte]

Created attachment 42858 [details]
updated ChangeLog entry

[Eric Seidel (no email)]

Comment on attachment 42858 [details]
updated ChangeLog entry

LGTM too.  You should be aware of:
prepare-ChangeLog --bug 12345
which will automatically add the title and url for the bug to the ChangeLog.

[WebKit Commit Bot]

Comment on attachment 42858 [details]
updated ChangeLog entry

Rejecting patch 42858 from commit-queue.

Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Eric Seidel', '--force']" exit_code: 1
Last 500 characters of output:
aphics/gtk/MediaPlayerPrivateGStreamer.cpp   |   51 ++++----------------
 2 files changed, 23 insertions(+), 42 deletions(-)


-------------------------------------------------------------------
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp
Hunk #1 FAILED at 634.
Hunk #2 FAILED at 655.
2 out of 2 hunks FAILED -- saving rejects to file WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp.rej

[Benjamin Otte]

Created attachment 42883 [details]
rebased against trunk

[Jan Alonzo]

Comment on attachment 42883 [details]
rebased against trunk

> [phone]  Benjamin Otte  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [GTK] Black artifacts in youtube.com/html5
> +
> +        Paint the video to the given size. It's the job of the callers to keep
> +        track of aspect ratio. RenderVideo.cpp does it for the <video>
> +        element.
> +        https://bugs.webkit.org/show_bug.cgi?id=30925
> +

Try to put the bug link after the title next time.

[WebKit Commit Bot]

Comment on attachment 42883 [details]
rebased against trunk

Clearing flags on attachment: 42883

Committed r50798: <http://trac.webkit.org/changeset/50798>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.