[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 #373695|none                        |needs-work
             status|                            |

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

This is getting a better shape, but still needs some love :)

::: gst-libs/gst/vaapi/gstvaapisurface.c
@@ +106,3 @@
+
+  width = GST_VIDEO_INFO_WIDTH (vip);
+  height = GST_VIDEO_INFO_HEIGHT (vip);

why do you need the whole GstVideoInfo structure to only keep using the same
width and height?

I don't understand.

@@ +347,3 @@
+    gst_video_info_set_format (&vi, GST_VIDEO_FORMAT_YUY2, width, height);
+  else
+    gst_video_info_set_format (&vi, GST_VIDEO_FORMAT_NV12, width, height);

I guess, the best approach to this is too add a new helper function in
video-format.c called, gst_vaapi_video_format_from_chroma() instead of this
branches.

Also this block should be guarded by  #if VA_CHECK_VERSION(0,34,0)

::: gst/vaapi/gstvaapipluginbase.c
@@ +1317,3 @@
+    gst_vaapi_object_unref (image);
+  }
+

This is code duplication, and it is bad pattern. It is better to refactor into
an internal helper function.

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