794061 - Copying video frame to canvas with B2G Omx backend enabled fails

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[cajbir (:cajbir)]

STR:

1) Enable B2G OMX Backend (Required bus 759506 and dependancies)
2) Copy an mp4 file to /sdcard/Movies
3) Start video app

Expected result:

4) Thumbnail of mp4 file is in list of videos

Actual result:

4) canvas drawImage call fails and thumbnail is not generated.

The Omx backend uses a zero-copy method of using the hardware decoders so probably fails to get the pixel data. Reproduction of this bug is complicated by the recently introduced bug 794055 so I'll try to do a smaller test case for it soon.

Using the media plugin backend (media.plugins.enabled) does allow drawing of the thumbnails.

Comment 1

[cajbir (:cajbir)]

Edwin, can you investigate what's going on here please.

Comment 2

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: WIP

Works modulo some colour conversion funniness. Our software YUV->RGB conversion doesn't support semi-planar images so colours are funky on the images I've seen from the OMX decoder so far. Looking into bug 791941 to see if we can throw out the existing conversion code we lifted from Chromium long ago and slot in libyuv (like Chromium does nowadays).

Comment 3

[Sotaro Ikeda [:sotaro]]

Android stagefright has ColorConverter class. It could be used to convert YUV->RGB. StagefrightMetadataRetriever uses it to generate Video thumbnail images. Therefore the ColorConverter is exteneded by vendor to support proprietary color formats.

My phone's(ics qcomm) hw codec uses following color format.
 - HAL_PIXEL_FORMAT_YCbCr_420_SP_TILED
    (OMX_QCOM_COLOR_FormatYUV420PackedSemiPlanar64x32Tile2m8ka)
It is very hard to convert to RGB withiout using stagefright's ColorConverter nor OpenGL. Within it, it uses "libmm-color-convertor.so".

https://www.codeaurora.org/gitweb/quic/la/?p=platform/frameworks/base.git;a=blob;f=media/libstagefright/colorconversion/ColorConverter.cpp;h=c0b07cf502faee0d0d9979ee1edc1972c7e9eccd;hb=ics_strawberry#l130

It seems that qualcomm use the color format when hw video codecs' performance is important. 

Following is just FYI.
http://stackoverflow.com/questions/10059738/qomx-color-formatyuv420packedsemiplanar64x32tile2m8ka-color-format

Comment 4

[Sotaro Ikeda [:sotaro]]

The ColorConverter uses OMX color format id. The id seems unique even when it is a proprietry color format. But GraphicBuffer uses android's pixcel format id. There might be possible that OEM proprietary format ID is not unique. Format id range is just (0x100 <= format <=0x1FF).

http://androidxref.com/4.0.4/xref/frameworks/base/libs/gui/SurfaceTexture.cpp#849

Comment 5

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Well. That makes things much easier.

Comment 6

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Patch

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 669008 [details] [diff] [review]
Patch

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

r+ with those fixed

::: gfx/layers/GonkIOSurfaceImage.cpp
@@ +45,5 @@
> +  GraphicBufferAutoUnlock unlock(graphicBuffer);
> +
> +  if (rv) {
> +    NS_WARNING("Couldn't lock graphic buffer");
> +    return nullptr;

This unlocks the buffer, which it shouldn't.

Move 'unlock' further down?

@@ +79,5 @@
> +  uint32_t width = GetSize().width;
> +  uint32_t height = GetSize().height;
> +
> +  if (omxFormat == OMX_QCOM_COLOR_FormatYVU420SemiPlanar) {
> +    // Fun fun alignment games

Please add more detailed documentation here

Comment 8

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/186dd6443aec

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Don't forget to request Aurora approval.

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/186dd6443aec

Comment 11

[:gerard-majax]

Great, I'll be able to finish my Gaia « Heart Rate » application thanks to this one fixed :)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/adfd438b56ff