854539 - AllocateAndGetNewBuffer need to be implemented for SharedPlanarYCbCrImage otherwise it crash

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Oleg Romashin (:romaxa)]

In bug 761018 we introduced PlanarYCbCrImage::AllocateAndGetNewBuffer
and no any implementation for SharedPlanarYCbCrImage, which makes async video with gstreamer backend crashy.

Comment 1

[Oleg Romashin (:romaxa)]

#0  mozilla::Mutex::Lock (this=0x4) at ../../../dist/include/mozilla/Mutex.h:74
#1  0x3b4188f0 in mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock (this=0x403f5b0c, aLock=...) at ../../../dist/include/mozilla/Mutex.h:153
#2  0x3c3c5de0 in mozilla::layers::BufferRecycleBin::GetBuffer (this=0x0, aSize=194400) at gfx/layers/ImageContainer.cpp:112
#3  0x3c3c49c4 in mozilla::layers::PlanarYCbCrImage::AllocateAndGetNewBuffer (this=0x0, aSize=4) at gfx/layers/ImageContainer.cpp:509
#4  0x3bc78ad0 in mozilla::GStreamerReader::AllocateVideoBufferFull (this=<value optimized out>, aPad=<value optimized out>, aOffset=<value optimized out>, aSize=194400, aCaps=0x0, aBuf=0x403f5c04, aImage=...)
    at content/media/gstreamer/GStreamerReader.cpp:794
#5  0x3bc79be4 in mozilla::GStreamerReader::DecodeVideoFrame (this=0x3f635208, aKeyFrameSkip=@0x403f5c47, aTimeThreshold=<value optimized out>)
    at content/media/gstreamer/GStreamerReader.cpp:495
#6  0x3bc56340 in mozilla::MediaDecoderReader::DecodeToFirstVideoData (this=0x3f635208) at content/media/MediaDecoderReader.cpp:417
#7  0x3bc564ac in mozilla::MediaDecoderReader::FindStartTime (this=0x3f635208, aOutStartTime=@0x403f5c80) at content/media/MediaDecoderReader.cpp:451
#8  0x3bc502f0 in mozilla::MediaDecoderStateMachine::FindStartTime (this=0x3f6600a8) at content/media/MediaDecoderStateMachine.cpp:2483
#9  0x3bc52e78 in mozilla::MediaDecoderStateMachine::DecodeMetadata (this=0x3f6600a8) at content/media/MediaDecoderStateMachine.cpp:1820
#10 0x3bc55bb8 in mozilla::MediaDecoderStateMachine::DecodeThreadRun (this=0x3f6600a8) at content/media/MediaDecoderStateMachine.cpp:480
#11 0x3bc50404 in nsRunnableMethodImpl<void (mozilla::MediaDecoderStateMachine::*)(), true>::Run (this=<value optimized out>) at ../../dist/include/nsThreadUtils.h:367
#12 0x3c32c420 in nsThread::ProcessNextEvent (this=0x6d56a0, mayWait=<value optimized out>, result=0x403f5d7f) at xpcom/threads/nsThread.cpp:627
#13 0x3c2f394c in NS_ProcessNextEvent_P (thread=0x3f6600a8, mayWait=true) at /home/romaxa/build/xulrunner-package/objdir-harmattan/xpcom/build/nsThreadUtils.cpp:238
#14 0x3c32b804 in nsThread::ThreadFunc (arg=<value optimized out>) at xpcom/threads/nsThread.cpp:265
#15 0x3ad83784 in _pt_root (arg=<value optimized out>) at nsprpub/pr/src/pthreads/ptthread.c:191
#16 0x4119d94c in start_thread (arg=<value optimized out>) at pthread_create.c:302
#17 0x41108868 in clone () at ../ports/sysdeps/unix/sysv/linux/arm/nptl/../clone.S:101


mozilla::GStreamerReader::AllocateVideoBufferFull (this=<value optimized out>, aPad=<value optimized out>, aOffset=<value optimized out>, aSize=194400, aCaps=0x0, aBuf=0x403f5c04, aImage=...)
    at content/media/gstreamer/GStreamerReader.cpp:794
794	  GST_BUFFER_DATA(buf) = image->AllocateAndGetNewBuffer(aSize);
(gdb) s

Breakpoint 2, mozilla::layers::PlanarYCbCrImage::AllocateAndGetNewBuffer (this=0xc77ed8, aSize=194400) at gfx/layers/ImageContainer.cpp:509
509	  mBuffer = AllocateBuffer(mBufferSize); 
(gdb) p this
$15 = (mozilla::layers::SharedPlanarYCbCrImage *) 0xc77ed8
(gdb) p *this
$16 = (mozilla::layers::SharedPlanarYCbCrImage) {<mozilla::layers::PlanarYCbCrImage> = {<mozilla::layers::Image> = {_vptr.Image = 0x3d369ac8, mRefCnt = {mValue = 1}, mBackendData = {{mRawPtr = 0x0}, {
          mRawPtr = 0x0}, {mRawPtr = 0x0}, {mRawPtr = 0x0}, {mRawPtr = 0x0}}, mImplData = 0x0, mSerial = 1, mFormat = mozilla::PLANAR_YCBCR, static sSerialCounter = 1, mSent = false}, mBuffer = {
      mRawPtr = 0x0}, mBufferSize = 0, mData = {mYChannel = 0x0, mYStride = 0, mYSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 0, height = 0}, <No data fields>}, mYSkip = 0, mCbChannel = 0x0, 
      mCrChannel = 0x0, mCbCrStride = 0, mCbCrSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 0, height = 0}, <No data fields>}, mCbSkip = 0, mCrSkip = 0, mPicX = 0, mPicY = 0, 
      mPicSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 0, height = 0}, <No data fields>}, mStereoMode = mozilla::STEREO_MODE_MONO}, mSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {
        width = 0, height = 0}, <No data fields>}, mOffscreenFormat = gfxASurface::ImageFormatUnknown, 
    mSurface = {<nsAutoRef<nsMainThreadSurfaceRef>> = {<nsAutoRefBase<nsMainThreadSurfaceRef>> = {<nsSimpleRef<nsMainThreadSurfaceRef>> = {<nsAutoRefTraits<nsMainThreadSurfaceRef>> = {<No data fields>}, 
            mRawRef = 0x0}, <No data fields>}, <No data fields>}, <No data fields>}, mRecycleBin = {mRawPtr = 0x0}}, mShmem = {mSegment = 0x0, mData = 0x0, mSize = 0, mId = 0}, mImageContainerChild = {
    mRawPtr = 0x96f098}, mAllocated = false}
(gdb) p mBuffer
$17 = {mRawPtr = 0x0}
(gdb) n
504	{
(gdb) n
506	  mBufferSize = aSize;
(gdb) p mBuffer
$18 = {mRawPtr = 0x0}
(gdb) n
509	  mBuffer = AllocateBuffer(mBufferSize); 
(gdb) s
mozilla::layers::PlanarYCbCrImage::AllocateBuffer (this=0xc77ed8, aSize=194400) at gfx/layers/ImageContainer.cpp:426
426	  return mRecycleBin->GetBuffer(aSize); 
(gdb) p mRecycleBin 
$19 = {mRawPtr = 0x0}
(gdb) c
Continuing.

Comment 2

[Nicolas Silva [:nical]]

Attachment: Implement AllocateBuffer and AllocateAndGetNewBuffer for SharedPlanarYCbCrImage.

try push: https://tbpl.mozilla.org/?tree=Try&rev=bd6d0154186b

Comment 3

[Oleg Romashin (:romaxa)]

it pass that stage but crashes later here
#0  0x3af95dec in mozalloc_abort (msg=<value optimized out>) at memory/mozalloc/mozalloc_abort.cpp:30
#1  0x3af95eac in mozalloc_handle_oom (size=0) at memory/mozalloc/mozalloc_oom.cpp:50
#2  0x3af95db4 in moz_xmalloc (size=1901585092) at memory/mozalloc/mozalloc.cpp:56
#3  0x3c3f4480 in operator new [] (this=0x45b386b8, target=0, level=123, xoffset=0, yoffset=0, width=1734962028, height=1751605859, stride=0, pixelsize=1, format=6409, type=5121, pixels=0x0)
    at ../../dist/include/mozilla/mozalloc.h:213
#4  mozilla::gl::GLContext::TexSubImage2DWithoutUnpackSubimage (this=0x45b386b8, target=0, level=123, xoffset=0, yoffset=0, width=1734962028, height=1751605859, stride=0, pixelsize=1, format=6409, type=5121, 
    pixels=0x0) at gfx/gl/GLContext.cpp:2359
#5  0x3c3f4634 in mozilla::gl::GLContext::TexSubImage2D (this=0xa, target=0, level=123, xoffset=0, yoffset=0, width=1734962028, height=1751605859, stride=0, pixelsize=1, format=6409, type=1, pixels=0x0)
    at gfx/gl/GLContext.cpp:2284
#6  0x3c3f4ca0 in mozilla::gl::GLContext::TexImage2D (this=0x45b386b8, target=<value optimized out>, level=<value optimized out>, internalformat=<value optimized out>, width=1734962028, height=1751605859, 
    stride=0, pixelsize=1, border=0, format=6409, type=5121, pixels=0x0) at gfx/gl/GLContext.cpp:2230
#7  0x3c3f5ebc in mozilla::gl::GLContext::UploadSurfaceToTexture (this=0x45b386b8, aSurface=<value optimized out>, aDstRegion=<value optimized out>, aTexture=<value optimized out>, 
    aOverwrite=<value optimized out>, aSrcPoint=..., aPixelBuffer=true, aTextureUnit=2932854824) at gfx/gl/GLContext.cpp:2127
#8  0x3c3cf40c in mozilla::layers::UploadYUVToTexture (gl=<value optimized out>, aData=..., aYTexture=<value optimized out>, aUTexture=<value optimized out>, aVTexture=0x409692b0)
    at gfx/layers/opengl/ImageLayerOGL.cpp:482
#9  0x3c3cf718 in mozilla::layers::ShadowImageLayerOGL::UploadSharedYCbCrToTexture (this=0x40969050, aImage=..., aPictureRect=<value optimized out>)
    at gfx/layers/opengl/ImageLayerOGL.cpp:925
#10 0x3c3cfe00 in mozilla::layers::ShadowImageLayerOGL::RenderLayer (this=0x40969050, aPreviousFrameBuffer=<value optimized out>, aOffset=...)
    at gfx/layers/opengl/ImageLayerOGL.cpp:982
#11 0x3c3cd604 in mozilla::layers::ContainerRender<mozilla::layers::ShadowContainerLayerOGL> (aContainer=0x79a7e0, aPreviousFrameBuffer=0, aOffset=<value optimized out>, aManager=0x45b1d2d8)
---Type <return> to continue, or q <return> to quit---
    at gfx/layers/opengl/ContainerLayerOGL.cpp:274
#12 0x3c3d4210 in mozilla::layers::LayerManagerOGL::Render (this=0x45b1d2d8) at gfx/layers/opengl/LayerManagerOGL.cpp:1131
#13 0x3c3d46f8 in mozilla::layers::LayerManagerOGL::EndTransaction (this=0x45b1d2d8, aCallback=0, aCallbackData=0x0, aFlags=<value optimized out>)
    at gfx/layers/opengl/LayerManagerOGL.cpp:799
#14 0x3c3d137c in mozilla::layers::LayerManagerOGL::EndEmptyTransaction (this=<value optimized out>, aFlags=mozilla::layers::LayerManager::END_DEFAULT)
    at gfx/layers/opengl/LayerManagerOGL.cpp:740
#15 0x3c3e956c in mozilla::layers::CompositorParent::Composite (this=0xa9ce50) at gfx/layers/ipc/CompositorParent.cpp:599

Comment 4

[Oleg Romashin (:romaxa)]

#9  0x3c3cf718 in mozilla::layers::ShadowImageLayerOGL::UploadSharedYCbCrToTexture (this=0x40969050, aImage=..., aPictureRect=<value optimized out>)
    at gfx/layers/opengl/ImageLayerOGL.cpp:925
925	  UploadYUVToTexture(gl(), data, &mYUVTexture[0], &mYUVTexture[1], &mYUVTexture[2]);
(gdb) p data
$2 = {mYChannel = 0xacf18464 <Address 0xacf18464 out of bounds>, mYStride = 1734962028, mYSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 1734962028, height = 1751605859}, <No data fields>}, 
  mYSkip = 0, mCbChannel = 0xb7f98664 <Address 0xb7f98664 out of bounds>, mCrChannel = 0xb6fa8d6e <Address 0xb6fa8d6e out of bounds>, mCbCrStride = 1936878698, 
  mCbCrSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 1936878698, height = 1735028595}, <No data fields>}, mCbSkip = 0, mCrSkip = 0, mPicX = 0, mPicY = 0, 
  mPicSize = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 0, height = 0}, <No data fields>}, mStereoMode = mozilla::STEREO_MODE_MONO}

Comment 5

[Nicolas Silva [:nical]]

hm, actually it is not as trivial as I thought. The problem is that before the gstreamer patch, we would always have a bit more information when allocating the ycbcr buffers, and the shared version kinda stores image data with it's own layout and adds a bit of metadata at the beginning of the buffer at allocation time. So we need to make sure that we support in SharedPlanarYCbCrImage the layout that GStreamer uses, which should not be a problem, and make it so we can allocate the buffer and add the metadata separately. It is not hugely complicated but right now I need to spend most of my time on the layers refactoring.
This bug should not be marked critical because it only affects a configuration that is not officially supported (OMTC on linux platform with gstreamer). It will have to wait that either I finish what I am doing with the layers refactoring or that someone else takes the bug.
A quick (and sad) workaround could be to not activate async-video on the platform that gets this bug until someone has time to fix this properly...

Comment 6

[Oleg Romashin (:romaxa)]

> A quick (and sad) workaround could be to not activate async-video on the
> platform that gets this bug until someone has time to fix this properly...

that is what I did, but hope that is temporary solution

Comment 7

[Ivaylo Dimitrov]

Attachment: patch.diff

Based on the first patch but also implements metadata allocation and SetDataNoCopy to initialize metadata. Tested with that patch on the device, so far, so good.

Comment 8

[Nicolas Silva [:nical]]

Comment on attachment 732042 [details] [diff] [review]
patch.diff

Review of attachment 732042 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't do a thorough review, but it looks good to me. It should still get a proper review though.

Comment 9

[Ivaylo Dimitrov]

Attachment: AllocateAndGetNewBuffer and metadata init for SharedPlanarYCbCrImage

fixed the source files file permissions

Comment 10

[Nicolas Silva [:nical]]

Comment on attachment 733370 [details] [diff] [review]
AllocateAndGetNewBuffer and metadata init for SharedPlanarYCbCrImage

Review of attachment 733370 [details] [diff] [review]:
-----------------------------------------------------------------

It looks good to me. Let's make sure we have a green try push before landing:
https://tbpl.mozilla.org/?tree=Try&rev=89fadcc4094e

::: gfx/layers/ipc/ImageContainerChild.cpp
@@ +589,3 @@
>  
> +  virtual void
> +  SetDataNoCopy(const Data &aData)

nit: It's a good practice to mark methods as MOZ_OVERRIDE whenever applicable.

Comment 11

[Nicolas Silva [:nical]]

Also, I didn't catch it during the review, but you should format your patch with the right hg header and commit message as explained here:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

I'll do it this time :)

Comment 12

[Nicolas Silva [:nical]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1aa5e2d8cd87

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1aa5e2d8cd87