After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 651439 - Wrong video raw format fourcc assignment for I420
Wrong video raw format fourcc assignment for I420
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
unspecified
Other All
: Normal major
: 0.10.2
Assigned To: Felipe Contreras (banned)
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-05-30 08:55 UTC by kan.hu
Modified: 2011-10-29 15:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch for fix I420 openmax mapping. (523 bytes, patch)
2011-05-30 13:36 UTC, kan.hu
none Details | Review

Description kan.hu 2011-05-30 08:55:59 UTC
video raw I420 format for openmax OMX_COLOR_FORMATTYPE should be OMX_COLOR_FormatYUV420Planar. 
Please see settings_changed_cb in omx/omx/gstomx_base_videodec.c
Attached patch to fix this issue.
Comment 1 Felipe Contreras (banned) 2011-05-30 10:25:55 UTC
The specification mentions the following:

OMX_COLOR_FormatYUV420Planar 

YUV planar format, organized with three separate planes for each color
component, namely Y, U, and V appearing in this order. U and V pixels are
sub-sampled by a factor of two both horizontally and vertically.

OMX_COLOR_FormatYUV420PackedPlanar 

YUV planar format, organized with three separate planes for each color
component, namely Y, U, and V. U and V pixels are sub-sampled by a factor of
two both horizontally and vertically. This format differs from
OMX_COLOR_FormatYUV420Planar in that each slice of data shall contain a plane
of Y, U, and V data in this order, whereas the OMX_COLOR_FormatYUV420Planar
format transfers each plane in its entirety.

Although that is a bit confusing, I thought that the "Packed" version is closer to what GStreamer does; disregard all slices and planes and send everything in one big chunk. Isn't that the case?
Comment 2 kan.hu 2011-05-30 12:19:13 UTC
Yes. But I think maybe both OMX_COLOR_FormatYUV420Planar and  OMX_COLOR_FormatYUV420PackedPlanar should be mapped to I420 format. I have tested gst-openmax with bellagio mpeg4 and h264 decoder, which both report raw video color-format as OMX_COLOR_FormatYUV420Planar.
Comment 3 Felipe Contreras (banned) 2011-05-30 12:58:38 UTC
(In reply to comment #2)
> Yes. But I think maybe both OMX_COLOR_FormatYUV420Planar and 
> OMX_COLOR_FormatYUV420PackedPlanar should be mapped to I420 format. I have
> tested gst-openmax with bellagio mpeg4 and h264 decoder, which both report raw
> video color-format as OMX_COLOR_FormatYUV420Planar.

Ok, well, I changed that because TI switched to PackedPlanar, and reading from the omx spec I agreed that was correct. I remember sending the patches to bellagio, and they also agreed it was correct, but perhaps they forgot, I don't know.

Anyway, I don't mind mapping both to I420, because GStreamer has no notion of planes anyway, but if at some point GStreamer adds that support, then we are in trouble, and we would need to fix the omx implementations.

BTW. I don't see any patch.
Comment 4 kan.hu 2011-05-30 13:36:44 UTC
Created attachment 188884 [details] [review]
proposed patch for fix I420 openmax mapping.
Comment 5 kan.hu 2011-05-30 13:38:16 UTC
I have created a patch, please have a review. thanks.
Comment 6 Felipe Contreras (banned) 2011-05-30 15:28:45 UTC
(In reply to comment #4)
> Created an attachment (id=188884) [details] [review]
> proposed patch for fix I420 openmax mapping.

Looks fine, but please use 'git format-patch' to generate your patches from a commit, with a summary and commit message. This time I wrote one for you:

---
From: Kan Hu <kan.hu@linaro.org>
Subject: [PATCH] base_videodec: workaround for non-packed I420 type

Some people use OMX_COLOR_FormatYUV420Planar, some
OMX_COLOR_FormatYUV420PackedPlanar. Strictly speaking the format needed
in GStreamer is the packed one, but since there's no notion of planes in
GStreamer, considering them the same cannot hurt.

Would be better if the implementations are fixed, specially if we
implement stride/plane support in GStreamer/gst-openmax.
---

Applied and pushed.