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 758699 - Assume unknown OMX color formats are similar to YUV420
Assume unknown OMX color formats are similar to YUV420
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-26 12:02 UTC by Mathias Hasselmann (IRC: tbf)
Modified: 2018-11-03 13:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (1.62 KB, patch)
2015-11-26 12:04 UTC, Mathias Hasselmann (IRC: tbf)
none Details | Review

Description Mathias Hasselmann (IRC: tbf) 2015-11-26 12:02:31 UTC
Android hardware decoders usually report undocumented OMX color formats resulting in the accelerated pipeline not building up. I believe it's safe to assume that those unknown color formats are similar to COLOR_FormatYUV420Planar and only print a warning without bailing out. 99.5% this will produce happy users with proper video, remaining 0.5% hopefully file a bug report.
Comment 1 Mathias Hasselmann (IRC: tbf) 2015-11-26 12:04:07 UTC
Created attachment 316306 [details] [review]
Proposed patch
Comment 2 Nicolas Dufresne (ndufresne) 2015-11-26 13:34:40 UTC
That seems a very crash prone idea for the case where it's not.
Comment 3 Mathias Hasselmann (IRC: tbf) 2015-11-26 14:33:41 UTC
(In reply to Nicolas Dufresne (stormer) from comment #2)
> That seems a very crash prone idea for the case where it's not.

Yes, still we have to find a way to deal with all the undefined color formats reported by all this real hardware out in the wild. Actually could not find a single Android device that had a H.264 hardware decoder reporting any supported color format.

Well, guess I should add all the well known formats to the switch statement. Any maybe we can have some sanity checks, that verify if the buffer has exactly the expected size, as indicated by the chosen color format.
Comment 4 Nicolas Dufresne (ndufresne) 2015-11-26 15:10:17 UTC
(In reply to Mathias Hasselmann (IRC: tbf) from comment #3)
> (In reply to Nicolas Dufresne (stormer) from comment #2)
> > That seems a very crash prone idea for the case where it's not.
> 
> Yes, still we have to find a way to deal with all the undefined color
> formats reported by all this real hardware out in the wild. Actually could
> not find a single Android device that had a H.264 hardware decoder reporting
> any supported color format.

Yes, that way already exist, it's called texture rendering. We are already working on this.

> 
> Well, guess I should add all the well known formats to the switch statement.
> Any maybe we can have some sanity checks, that verify if the buffer has
> exactly the expected size, as indicated by the chosen color format.

For the mappable memory, the appropriate thing to do is report all the values that are found and map them. There is additional meta-data for most of these I420 and NV12 (I believe it's half NV12 and half I420 unlike what you mention). The size and the stride of I420 and NV12 is exactly the same.
Comment 5 Sebastian Dröge (slomo) 2015-11-26 16:57:09 UTC
(In reply to Nicolas Dufresne (stormer) from comment #4)
> (In reply to Mathias Hasselmann (IRC: tbf) from comment #3)
> > (In reply to Nicolas Dufresne (stormer) from comment #2)
> > > That seems a very crash prone idea for the case where it's not.
> > 
> > Yes, still we have to find a way to deal with all the undefined color
> > formats reported by all this real hardware out in the wild. Actually could
> > not find a single Android device that had a H.264 hardware decoder reporting
> > any supported color format.
> 
> Yes, that way already exist, it's called texture rendering. We are already
> working on this.

And that is already finished now in gst-plugins-bad GIT with Matthew's latest changes. Only needs some wider testing :)

> > Well, guess I should add all the well known formats to the switch statement.
> > Any maybe we can have some sanity checks, that verify if the buffer has
> > exactly the expected size, as indicated by the chosen color format.
> 
> For the mappable memory, the appropriate thing to do is report all the
> values that are found and map them. There is additional meta-data for most
> of these I420 and NV12 (I believe it's half NV12 and half I420 unlike what
> you mention). The size and the stride of I420 and NV12 is exactly the same.

Yes I also dislike this idea. And if anything we should probably use NV12 instead of I420, NV12 seems to be slightly more common.
But in general this is not a good idea, instead we should report an error message and let the user report a bug directly. Otherwise users will just see that "GStreamer does not work" instead of getting the information that it's just Android that is providing completely useless APIs.
Comment 6 Mathias Hasselmann (IRC: tbf) 2015-11-27 08:53:23 UTC
(In reply to Sebastian Dröge (slomo) from comment #5)
> (In reply to Nicolas Dufresne (stormer) from comment #4)
> > (In reply to Mathias Hasselmann (IRC: tbf) from comment #3)
> > > (In reply to Nicolas Dufresne (stormer) from comment #2)
> > > > That seems a very crash prone idea for the case where it's not.
> > > 
> > > Yes, still we have to find a way to deal with all the undefined color
> > > formats reported by all this real hardware out in the wild. Actually could
> > > not find a single Android device that had a H.264 hardware decoder reporting
> > > any supported color format.
> > 
> > Yes, that way already exist, it's called texture rendering. We are already
> > working on this.
> 
> And that is already finished now in gst-plugins-bad GIT with Matthew's
> latest changes. Only needs some wider testing :)

Is there a branch for this work? For me master still rejects any OMX_COLOR_FormatAndroidOpaque format (0x7F000789).

I'd also like to point out that master consumes 2.4 the CPU time of pure Android Media or even QtMultimedia solutions still. So there is room for improvement.

> > > Well, guess I should add all the well known formats to the switch statement.
> > > Any maybe we can have some sanity checks, that verify if the buffer has
> > > exactly the expected size, as indicated by the chosen color format.
> > 
> > For the mappable memory, the appropriate thing to do is report all the
> > values that are found and map them. There is additional meta-data for most
> > of these I420 and NV12 (I believe it's half NV12 and half I420 unlike what
> > you mention). The size and the stride of I420 and NV12 is exactly the same.
> 
> Yes I also dislike this idea. And if anything we should probably use NV12
> instead of I420, NV12 seems to be slightly more common.
> But in general this is not a good idea, instead we should report an error
> message and let the user report a bug directly. Otherwise users will just
> see that "GStreamer does not work" instead of getting the information that
> it's just Android that is providing completely useless APIs.

I fully agree, that my approach above is too simplistic and broken. Alone because it claims to certainty about the video format while it was guessing, potentially causing equivalent, but properly recognized formats to get rejected. This is plain wrong and needs to be fixed for a proper patch.

I also agree, that within controlled environments like embedded platforms being strict about formats is the proper approach. Predictable behavior is the key to such projects and like Sebastian points out, missing features, slow code paths are found quickly in such environment.

Now let's enter the world of Android, sometimes used as embedded platform, were all the assumptions about strict error handling apply. Sometimes just a wild zoo of consumer hardware, with new devices, new chipsets popping up "every other week". In that scenario it seems more beneficial to me be fault tolerant and to run a best effort approach to support even the most exotic animals of the zoo. That mindset is where this bug report comes from.

Now the question is, what Android devices GStreamer targets. Does it aim for the embedded use case only, or does it also target Play Store applications. In first case this bug report is just invalid, in the second case we should come up with a solution for doing a best effort approach to identify unknown formats.
Comment 7 Sebastian Dröge (slomo) 2015-11-27 13:56:49 UTC
(In reply to Mathias Hasselmann (IRC: tbf) from comment #6)
> 
> Is there a branch for this work? For me master still rejects any
> OMX_COLOR_FormatAndroidOpaque format (0x7F000789).
> 
> I'd also like to point out that master consumes 2.4 the CPU time of pure
> Android Media or even QtMultimedia solutions still. So there is room for
> improvement.

If you use playbin and glimagesink, it should handle this and not use a lot of CPU. You need GIT master though.

Example with my phone, 720p30 without zerocopy uses 100% CPU (well, it doesn't play in real time in any case). With the recent zerocopy work it plays smoothly and uses 4% CPU or something around that.


Also by using zerocopy, you don't care about the actual color format used. It's just OpenGL in the end then.

> Now the question is, what Android devices GStreamer targets. Does it aim for
> the embedded use case only, or does it also target Play Store applications.

There are already quite a few GStreamer applications in the app store ;) Does that answer your question?
Comment 8 GStreamer system administrator 2018-11-03 13:43:42 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/331.