Bug 28569 - [i965] IGN's flash-based video player crashes X
Summary: [i965] IGN's flash-based video player crashes X
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/intel (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium critical
Assignee: Chris Wilson
QA Contact: Xorg Project Team
URL: http://wii.ign.com/dor/objects/872155...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-16 01:54 UTC by Brian Rogers
Modified: 2010-06-17 06:47 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
gdb backtrace (3.29 KB, text/plain)
2010-06-16 01:54 UTC, Brian Rogers
no flags Details
Refactor put_image (11.60 KB, patch)
2010-06-17 03:44 UTC, Chris Wilson
no flags Details | Splinter Review

Description Brian Rogers 2010-06-16 01:54:48 UTC
Created attachment 36308 [details]
gdb backtrace

On Ubuntu Lucid + xorg-edgers, I often get a crash when IGN's video player finishes playing a video. I have also seen it crash when I close a window containing the video. I haven't seen any other website crash it, just IGN.

To reproduce, just load the URL and wait. The player will play and endless looping playlist and will eventually crash the system.

I'm attaching a backtrace from gdb with my self-compiled DDX.
Comment 1 Brian Rogers 2010-06-17 03:05:44 UTC
9c3da71349bcfeabae08f1572cf602c357bf7641 is the first bad commit

commit 9c3da71349bcfeabae08f1572cf602c357bf7641
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sat May 15 01:05:24 2010 +0100

    i830: Remove xrgb conversion to argb, no longer required.
    
    All textures are now properly declared so that the alpha swizzling
    occurs in the sampler or not at all. The downside is that for quite a
    few composite operations we have to fallback to software on older
    hardware. There is scope for more performing the alpha expansion in
    shaders or combiners when we know the picture covers the clip - which is
    almost all of the time for normal operations especially those
    constructed by Cairo.
    
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Comment 2 Chris Wilson 2010-06-17 03:44:36 UTC
Created attachment 36326 [details] [review]
Refactor put_image

Wasn't intended to fix this, but it might, or rather I think it will trigger an EFAULT causing us to take a fallback, which is more conservative. I think the issue may lie an incomplete last row.

Changing the i830_bo_put_image() from:

  if (src_pitch==stride)
    memcpy(bo->virtual, src, stride * h);
to

  if (src_pitch == stride)
     memcpy (bo->virtual, src, stride * (h-1) + w *pixmap->drawable.bitsPerPixel/8);

would be informative.
Comment 3 Brian Rogers 2010-06-17 04:39:42 UTC
"two bugs one patch" :)

Your patch appears to solve both this bug and bug 28573.
Comment 4 Chris Wilson 2010-06-17 05:14:15 UTC
(In reply to comment #3)
> "two bugs one patch" :)
> 
> Your patch appears to solve both this bug and bug 28573.

I'll take that as success then!

May I use your tested-by?
Comment 5 Brian Rogers 2010-06-17 05:46:34 UTC
Yeah, go ahead.
Comment 6 Chris Wilson 2010-06-17 06:47:18 UTC
commit 0e0101758438debf98f989d815989b45e78cf5f6
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Jun 15 12:48:57 2010 +0100

    i830: Tidy i830_uxa_put_image()
    
    Use a single code path to upload the image data after selecting the
    right bo, and take advantage of pwrite() when possible.
    
    Fixes:
    
      Bug 28569 - [i965] IGN's flash-based video player crashes X
      https://bugs.freedesktop.org/show_bug.cgi?id=28569
    
      Bug 28573 - [i965] Fullscreen flash and windowed SDL games fail to
                  update the screen
      https://bugs.freedesktop.org/show_bug.cgi?id=28573
    
    Reported-and-tested-by: Brian Rogers <brian@xyzw.org>
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.