Closed Bug 794061 Opened 7 years ago Closed 7 years ago

Copying video frame to canvas with B2G Omx backend enabled fails

Categories

(Core :: Audio/Video, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: cajbir, Assigned: eflores)

References

(Blocks 1 open bug)

Details

(Whiteboard: [LOE:S])

Attachments

(1 file, 1 obsolete file)

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.
blocking-basecamp: --- → ?
Whiteboard: [LOE:S]
Assignee: nobody → chris.double
blocking-basecamp: ? → +
Edwin, can you investigate what's going on here please.
Assignee: chris.double → eflores
Attached patch WIP (obsolete) — Splinter Review
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).
Blocks: 798448
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
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
Well. That makes things much easier.
Attached patch PatchSplinter Review
Attachment #666865 - Attachment is obsolete: true
Attachment #669008 - Flags: review?(kchen)
Attachment #669008 - Flags: review?(roc)
Attachment #669008 - Flags: review?(kchen)
Attachment #669008 - Flags: review+
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
Attachment #669008 - Flags: review?(roc) → review+
Don't forget to request Aurora approval.
https://hg.mozilla.org/mozilla-central/rev/186dd6443aec
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Great, I'll be able to finish my Gaia « Heart Rate » application thanks to this one fixed :)
Whiteboard: [LOE:S] → [LOE:S][needs-checkin-aurora]
https://hg.mozilla.org/releases/mozilla-aurora/rev/adfd438b56ff
Flags: in-testsuite-
Whiteboard: [LOE:S][needs-checkin-aurora] → [LOE:S]
Blocks: 804906
Blocks: 806766
No longer depends on: 791941
No longer blocks: 787812
Blocks: 904069
You need to log in before you can comment on or make changes to this bug.