Closed Bug 1040648 Opened 10 years ago Closed 10 years ago

[Flatfish] Video Player crashes

Categories

(Firefox OS Graveyard :: Gaia::Video, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: nilaybinjola, Unassigned)

References

Details

(Whiteboard: [Flatfish][TCP=breakage])

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release) Build ID: 20140608211622 Steps to reproduce: 1. Get the FLATFISH "stable" directory image B2G 2.1.0.0-prerelease. 2. Insert SD card with some videos 3. Start Video Player application Actual results: It crashes with a message asking to send a crash report. Expected results: The video player should start.
Whiteboard: [Flatfish]
Blocks: flatfish
Crash Reason is a Segmentation Fault (SIGSEV) Here is the extracted crash report: https://crash-stats.mozilla.com/report/index/9c3998fc-0749-4fe7-86b2-ccca32140722 I think I did successfully flash the latest Gaia on the device.
Same here with build id 20140725020927
Make an app with this in the HTML; <video id="player" width="400" height="200" controls autoplay> <source src="small.webm" type="video/webm" /> </video> small.webm is a sample WebM file I got from http://techslides.com/sample-webm-ogg-and-mp4-video-files-for-html5/ Install the app on the Flatfish tablet and run it. Result: The app will crash. If anyone can tell me where I can get a crash report from I could possibly provide more information.
(In reply to broederjacobs from comment #5) > Make an app with this in the HTML; > <video id="player" width="400" height="200" controls autoplay> > <source src="small.webm" type="video/webm" /> > </video> > small.webm is a sample WebM file I got from > http://techslides.com/sample-webm-ogg-and-mp4-video-files-for-html5/ > > Install the app on the Flatfish tablet and run it. > > Result: The app will crash. > > If anyone can tell me where I can get a crash report from I could possibly > provide more information. This probably means that the crash has nothing to do with the Video player app, but just with the implementation of WebM in the HTML5 video element. I cannot reproduce this with an MP4 file (which has other problems).
@broederjacobs You can get the crash report quite easily by simply following the steps here: https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#Getting_crashes_off_the_Device I have already provided a crash report generated by just launching the app. Maybe yours can add to it. BTW, the bug is related to the Video Player App itself.
I confirm it probably comes from the Video Player App itself, as some webm files can be played correctly through the web browser of FxOS (in an HTML5 tag or by directly pointing to a webm file) : try the one from http://www.webmfiles.org/demo-files/
Or, at least, it might come from the way videos are handled in applications in general, by FxOS. Else your sample application would not crash
Same crash on build 20140803012528
Could you confirm this bug?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [Flatfish] → [Flatfish][TCP=breakage]
Video App crash because it use canvas.drawImage() create preview Image from video tag. In H.264 Video source playback The MediaOmxReader use VideoData* VideoData::Create(VideoInfo& aInfo, ImageContainer* aContainer, int64_t aOffset, int64_t aTime, int64_t aDuration, mozilla::layers::TextureClient* aBuffer, bool aKeyframe, int64_t aTimecode, const IntRect& aPicture) to Create mozilla::layers::GrallocImage and set data by GrallocImage::SetData(const GrallocData& aData) In GrallocImages.cpp, the canvas.drawImage() use TemporaryRef<gfx::SourceSurface> GrallocImage::GetAsSourceSurface() to render the picture There is an error when transform color space from HAL_PIXEL_FORMAT_YV12(YUV420) to SurfaceFormat::R5G6B5(RGB565) in ConvertOmxYUVFormatToRGB565(). It try to use void ConvertYCbCrToRGB(const layers::PlanarYCbCrData& aData, const SurfaceFormat& aDestFormat, const IntSize& aDestSize, unsigned char* aDestBuffer, int32_t aStride) to do color space transform. But if it use GrallocImage::SetData(const GrallocData& aData) to set data, it doesn't set layers::PlanarYCbCrData& aData (mData in GrallocImage), so it get Segmentation Fault by null point access when it try to do color space transform.
I add some code in GrallocImages.cpp to fix this problem. Code: At GrallocImage::GetAsSourceSurface() -> GrallocImages.cpp:385 if(mData.mYSize.width == 0 && mData.mYSize.height == 0){ uint8_t *buffer; rv = graphicBuffer->lock(android::GraphicBuffer::USAGE_SW_READ_OFTEN, reinterpret_cast<void **>(&buffer)); if (rv == OK) { // Android YV12 format is defined in system/core/include/system/graphics.h mData.mYChannel = static_cast<uint8_t*>(buffer); mData.mYSkip = 0; mData.mYSize.width = graphicBuffer->getWidth(); mData.mYSize.height = graphicBuffer->getHeight(); mData.mYStride = graphicBuffer->getStride(); // 4:2:0. mData.mCbCrSize.width = mData.mYSize.width / 2; mData.mCbCrSize.height = mData.mYSize.height / 2; mData.mCrChannel = mData.mYChannel + (mData.mYStride * mData.mYSize.height); // Aligned to 16 bytes boundary. mData.mCbCrStride = (mData.mYStride / 2 + 15) & ~0x0F; mData.mCrSkip = 0; mData.mCbChannel = mData.mCrChannel + (mData.mCbCrStride * mData.mCbCrSize.height); mData.mCbSkip = 0; mData.mPicSize = mSize; } graphicBuffer->unlock(); } Is it OK for Gecko !?
The above code seems not OK for gecko for code consistency point of view. You might want to fix ConvertOmxYUVFormatToRGB565(). Fixing the error of ConvertOmxYUVFormatToRGB565() in comment 14 seems a way to go.
I modify my code and move it to "ConvertOmxYUVFormatToRGB565() -> GrallocImages.cpp:319" Code: if (format == HAL_PIXEL_FORMAT_YV12) { if(aYcbcrData.mYSize.width == 0 && aYcbcrData.mYSize.height == 0) { layers::PlanarYCbCrData yuvBuf; // Android YV12 format is defined in system/core/include/system/graphics.h yuvBuf.mYChannel = static_cast<uint8_t*>(buffer); yuvBuf.mYSkip = 0; yuvBuf.mYSize.width = aBuffer->getWidth(); yuvBuf.mYSize.height = aBuffer->getHeight(); yuvBuf.mYStride = aBuffer->getStride(); // 4:2:0. yuvBuf.mCbCrSize.width = yuvBuf.mYSize.width / 2; yuvBuf.mCbCrSize.height = yuvBuf.mYSize.height / 2; yuvBuf.mCrChannel = yuvBuf.mYChannel + (yuvBuf.mYStride * yuvBuf.mYSize.height); // Aligned to 16 bytes boundary. yuvBuf.mCbCrStride = (yuvBuf.mYStride / 2 + 15) & ~0x0F; yuvBuf.mCrSkip = 0; yuvBuf.mCbChannel = yuvBuf.mCrChannel + (yuvBuf.mCbCrStride * yuvBuf.mCbCrSize.height); yuvBuf.mCbSkip = 0; yuvBuf.mPicSize = aYcbcrData.mPicSize; gfx::ConvertYCbCrToRGB(yuvBuf, aSurface->GetFormat(), aSurface->GetSize(), aSurface->GetData(), aSurface->Stride()); }else{ gfx::ConvertYCbCrToRGB(aYcbcrData, aSurface->GetFormat(), aSurface->GetSize(), aSurface->GetData(), aSurface->Stride()); } return OK; } I test it on my flatfish, The H.264 playback is fine.
(In reply to Joe Young from comment #17) > if(aYcbcrData.mYSize.width == 0 && aYcbcrData.mYSize.height == 0) { It seems better to add a comment why the above check is necessary.
(In reply to Sotaro Ikeda [:sotaro] from comment #19) > (In reply to Joe Young from comment #17) > > > if(aYcbcrData.mYSize.width == 0 && aYcbcrData.mYSize.height == 0) { > > It seems better to add a comment why the above check is necessary. Because it still can work, if use GrallocImage::SetData(const Data& aData) to set data. GrallocImage::SetData(const Data& aData) will initial mData in GrallocImage. -> mData == aYcbcrData So I use that to see aYcbcrData is exist or not
HAL_PIXEL_FORMAT_YV12 color format does two different color conversions depend on the condition, therefore the comment about the reason becomes necessary.
I found that Almost same patch is on going at Bug 1050192.
(In reply to Sotaro Ikeda [:sotaro] from comment #22) > I found that Almost same patch is on going at Bug 1050192. The patch was moved to Bug 1060811.
Depends on: 1060811
It sounds good !!
I will follow Bug 1060811
I just installed the 20140911 build and it now WFM - presumably due to bug 1060811.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.