[Bug 783114] omxh264dec: pass profile and level info to decoder

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=783114

Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #352630|none                        |needs-work
             status|                            |

--- Comment #9 from Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> ---
Review of attachment 352630:
 --> (https://bugzilla.gnome.org/review?bug=783114&attachment=352630)

::: omx/gstomxh264dec.c
@@ +138,3 @@
+  if (err != OMX_ErrorNone) {
+    GST_WARNING_OBJECT (self,
+        "Setting profile/level not supported by component");

This message is wrong, you call get_parameter and warn the setting the profile
is not supported. Also, calling get_parameter here without first providing
PPS/SPS is useless, if by chance you get a value, it's just going to be random.
If what you wanted is to iterate the supported value, you'd need to try
OMX_IndexParamVideoProfileLevel
QuerySupported instead.

@@ +143,3 @@
+
+  peercaps = gst_pad_peer_query_caps (GST_VIDEO_DECODER_SINK_PAD (dec),
+      gst_pad_get_pad_template_caps (GST_VIDEO_DECODER_SINK_PAD (dec)));

Maybe you can use gst_pad_get_allowed_caps() to simplify ?

@@ +175,3 @@
+  if (err == OMX_ErrorUnsupportedIndex) {
+    GST_WARNING_OBJECT (self,
+        "Setting profile/level not supported by component");

Just to illustrate how unusual this is, see OMX spec, table 4-61, and notice
that they say this is for "encoding". The usage of this paremeter for decoding
is not prohibited, but is a bit of a gray zone. It's pretty much undefined, and
I'm worried we'll endup issuing warnings on all the other platforms with hat
patch.

"OMX_IndexParamVideoProfileLevel:
Current eCompressionFormat, eProfile and eLevel will contain the requested
settiins to be used as part of the **encoding.** The nProfileIndex parameter is
an ignored parameter."

p.s. the settiins typo is from the spec, I think they wanted settings

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.