[Flatfish] Video Player crashes

RESOLVED WORKSFORME

Status

defect
RESOLVED WORKSFORME
5 years ago
5 years ago

People

(Reporter: nilaybinjola, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

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

Reporter

Description

5 years ago
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

5 years ago
Whiteboard: [Flatfish]
Reporter

Updated

5 years ago
Blocks: flatfish
Reporter

Comment 2

5 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.
Duplicate of this bug: 1041337

Comment 4

5 years ago
Same here with build id 20140725020927

Comment 5

5 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

5 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

5 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.

Comment 8

5 years ago
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/

Comment 9

5 years ago
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

5 years ago
Same crash on build 20140803012528
Duplicate of this bug: 1038873

Comment 12

5 years ago
Could you confirm this bug?
Me too: bp-2008d52b-7f4f-4912-a790-9a2282140819
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [Flatfish] → [Flatfish][TCP=breakage]

Comment 14

5 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

5 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 !?
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

5 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.
(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

5 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
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

Comment 24

5 years ago
It sounds good !!

Comment 25

5 years ago
I will follow Bug 1060811
I just installed the 20140911 build and it now WFM - presumably due to bug 1060811.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.