Closed
Bug 1040648
Opened 10 years ago
Closed 10 years ago
[Flatfish] Video Player crashes
Categories
(Firefox OS Graveyard :: Gaia::Video, defect)
Firefox OS Graveyard
Gaia::Video
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.
Reporter | ||
Updated•10 years ago
|
Whiteboard: [Flatfish]
Can you try on a more recent build and may we get the crash report please?
https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#Getting_crashes_off_the_Device
Reporter | ||
Comment 2•10 years ago
|
||
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.
Comment 5•10 years ago
|
||
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.
Comment 6•10 years ago
|
||
(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).
Reporter | ||
Comment 7•10 years ago
|
||
@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
Comment 10•10 years ago
|
||
Same crash on build 20140803012528
Comment 12•10 years ago
|
||
Could you confirm this bug?
Comment 13•10 years ago
|
||
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•10 years ago
|
Whiteboard: [Flatfish] → [Flatfish][TCP=breakage]
Comment 14•10 years ago
|
||
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.
Comment 15•10 years ago
|
||
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 !?
Comment 16•10 years ago
|
||
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.
Comment 17•10 years ago
|
||
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.
Comment 18•10 years ago
|
||
You might want to create a patch for a review. The followings are related to it.
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_diff_and_patch_files.3F
Comment 19•10 years ago
|
||
(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.
Comment 20•10 years ago
|
||
(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
Comment 21•10 years ago
|
||
HAL_PIXEL_FORMAT_YV12 color format does two different color conversions depend on the condition, therefore the comment about the reason becomes necessary.
Comment 22•10 years ago
|
||
I found that Almost same patch is on going at Bug 1050192.
Comment 23•10 years ago
|
||
(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.
Comment 24•10 years ago
|
||
It sounds good !!
Comment 25•10 years ago
|
||
I will follow Bug 1060811
Comment 26•10 years ago
|
||
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.
Description
•