[Bug 797143] vaapih265dec: support 422 chroma format hevc decode.

[GStreamer (GNOME Bugzilla)]

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

Víctor Manuel Jáquez Leal <vjaquez at igalia.com> changed:

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

--- Comment #2 from Víctor Manuel Jáquez Leal <vjaquez at igalia.com> ---
Review of attachment 373653:
 --> (https://bugzilla.gnome.org/review?bug=797143&attachment=373653)

::: gst-libs/gst/vaapi/gstvaapicontext.c
@@ +141,3 @@
+    else
+      surface = gst_vaapi_surface_new (GST_VAAPI_OBJECT_DISPLAY (context),
+          cip->chroma_type, cip->width, cip->height);

This changeset looks very hackish to me.

The proper way to handle this is in gstvaapidecoder_h265.c:ensure_context() by
setting the "proper" chroma type if YUV422 is detected.

But why cannot be created surfaces with that chroma type?

@@ +257,3 @@
   if (!context_get_attribute (context, attrib->type, &value))
     goto cleanup;
+  if (!(value && va_chroma_format)) {

No. You cannot convert a bitwise operation into a boolean operation.

::: gst-libs/gst/vaapi/gstvaapiprofile.h
@@ +132,3 @@
  * @GST_VAAPI_PROFILE_H265_MAIN_STILL_PICTURE:
  *   H.265 main still picture profile [A.3.4]
+  * @GST_VAAPI_PROFILE_H265_MAIN_422_10:

bad code style

@@ +178,3 @@
     GST_VAAPI_PROFILE_H265_MAIN_STILL_PICTURE =
                                                GST_VAAPI_MAKE_PROFILE(H265,3),
+     GST_VAAPI_PROFILE_H265_MAIN_422_10             =
GST_VAAPI_MAKE_PROFILE(H265,4),

ditto.

::: gst/vaapi/gstvaapidecode.c
@@ +88,3 @@
     GST_VAAPI_MAKE_GLTEXUPLOAD_CAPS ";"
 #endif
+    GST_VIDEO_CAPS_MAKE("{ NV12, I420, YV12, YUY2, UYVY, P010_10LE }") ";"

As far as I understand this changeset only applies to surfaces formats, not to
mapped images.

::: gst/vaapi/gstvaapipluginbase.c
@@ +1300,3 @@

+  if (out_formats->len == 0) {
+    GST_DEBUG ("No invalid image format found for surface!");

I guess the message has some typo or grammatical error. Perhaps it should be
"No valid image format found for surface" (no exclamation mark)

And perhaps it should be GST_WARNING.

But, the question is, do we really need that log message?

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