Bug 137065 – [GStreamer] Video resolution changes trigger a crash in the TextureMapper

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Philippe Normand]

When the video sink updates its caps the player is notified and clears its internal video size cache but this is not protected by a mutex so it's possible that the player tries to use caps that don't correspond to the buffer being rendered, triggering this crash:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:36
36	../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such file or directory.
(gdb) bt
#0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:36
#1  0x00007fe5bd010b5e in ?? () from /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so
#2  0x00007fe5bd015f20 in ?? () from /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so
#3  0x00007fe5bd0160d2 in ?? () from /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so
#4  0x00007fe5bd06a902 in ?? () from /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so
#5  0x00007fe5bd000e27 in ?? () from /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so
#6  0x00007fe5bd004b5f in ?? () from /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so
#7  0x00007fe623104f1a in WebCore::BitmapTextureGL::updateContentsNoSwizzle(void const*, WebCore::IntRect const&, WebCore::IntPoint const&, int, unsigned int, unsigned int) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007fe62310569a in WebCore::BitmapTextureGL::updateContents(void const*, WebCore::IntRect const&, WebCore::IntPoint const&, int, WebCore::BitmapTexture::UpdateContentsFlag) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007fe6230b891c in WebCore::MediaPlayerPrivateGStreamerBase::updateTexture(WebCore::TextureMapper*) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007fe6230b8a2f in WebCore::MediaPlayerPrivateGStreamerBase::paintToTextureMapper(WebCore::TextureMapper*, WebCore::FloatRect const&, WebCore::TransformationMatrix const&, float) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#11 0x00007fe622bbe3f3 in WebCore::TextureMapperLayer::paintSelf(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#12 0x00007fe622bc1dc2 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#13 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#14 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#15 0x00007fe622bc1d45 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) [clone .part.108] ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#16 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#17 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#18 0x00007fe622bc1d45 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) [clone .part.108] ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#19 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#20 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#21 0x00007fe622bc1d45 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) [clone .part.108] ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#22 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#23 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#24 0x00007fe622bc1d45 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) [clone .part.108] ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#25 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#26 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#27 0x00007fe622bc1d45 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) [clone .part.108] ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#28 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#29 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#30 0x00007fe622bc1d45 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) [clone .part.108] ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#31 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#32 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#33 0x00007fe622bc1d45 in WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) [clone .part.108] ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#34 0x00007fe622bc1f7d in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) ()
   from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#35 0x00007fe622bc1aa5 in WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#36 0x00007fe622bc1bc1 in WebCore::TextureMapperLayer::paint() () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#37 0x00007fe6223171a7 in WebKit::LayerTreeHostGtk::compositeLayersToContext(WebKit::LayerTreeHostGtk::CompositePurpose) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#38 0x00007fe622317710 in WebKit::LayerTreeHostGtk::flushAndRenderLayers() () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#39 0x00007fe6223177bb in WebKit::LayerTreeHostGtk::layerFlushTimerFired() () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#40 0x00007fe62048174c in WTF::GMainLoopSource::voidCallback() () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
---Type <return> to continue, or q <return> to quit---
#41 0x00007fe620481929 in WTF::GMainLoopSource::voidSourceCallback(WTF::GMainLoopSource*) () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#42 0x00007fe61db9c0fe in g_main_dispatch (context=0x13fdb10) at gmain.c:3065
#43 g_main_context_dispatch (context=context@entry=0x13fdb10) at gmain.c:3641
#44 0x00007fe61db9c4a8 in g_main_context_iterate (context=0x13fdb10, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3712
#45 0x00007fe61db9c912 in g_main_loop_run (loop=0x19c76b0) at gmain.c:3906
#46 0x00007fe62231b911 in WebProcessMainUnix () from /home/phil/dev/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#47 0x00007fe6207bfb45 in __libc_start_main (main=0x4006f0 <main>, argc=2, argv=0x7fffe3e8fd28, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffe3e8fd18)
    at libc-start.c:287
#48 0x0000000000400723 in _start ()

[Philippe Normand]

One workaround is to clear m_buffer when video caps changed but the side effect is a quick flash during the video size transition...

[Philippe Normand]

The proper fix would be to have DRAIN query support in the sink but currently only the OMX video decoder supports this, see also: https://bugzilla.gnome.org/show_bug.cgi?id=737240

[Sebastian Dröge (slomo)]

Or you always pass caps and buffer around at the same time? You could use GstSample for that

[Philippe Normand]

Created attachment 240473 [details]
patch

[Sebastian Dröge (slomo)]

Comment on attachment 240473 [details]
patch

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

Looks good overall and seems like a little simplification of the code :)

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:421
> +    GRefPtr<GstCaps> currentCaps = gst_sample_get_caps(m_sample);

Doesn't this need the sampleMutex locked too?

[Philippe Normand]

(In reply to comment #5)
> Comment on attachment 240473 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240473&action=review
> 
> Looks good overall and seems like a little simplification of the code :)
> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:421
> > +    GRefPtr<GstCaps> currentCaps = gst_sample_get_caps(m_sample);
> 
> Doesn't this need the sampleMutex locked too?

Well that method is always invoked with the mutex locked already. Perhaps I can add an ASSERT about this.

[Philippe Normand]

Or drop this method all together :)

[Philippe Normand]

Created attachment 240485 [details]
patch

[Philippe Normand]

Oops the sample caps don't really need to be stored in a GRefPtr.

[Philippe Normand]

Created attachment 240486 [details]
patch

Please review this one :)

[Gustavo Noronha (kov)]

Comment on attachment 240486 [details]
patch

Great =)

[Philippe Normand]

Committed r175370: <http://trac.webkit.org/changeset/175370>

[Xavier Claessens]

*** Bug 138505 has been marked as a duplicate of this bug. ***