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

[GStreamer (GNOME Bugzilla)]

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

--- Comment #14 from Fei <fei.w.wang at intel.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #13)
> Review of attachment 373695 [details] [review]:
> 
> 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.
This change is for fix of your previous comment:
>>This snippet has a couple problems, for example, if libva<0.34, vi is not >>used, so this computation is useless.
This make sure vi can be used in libva>=0.34 and libva<0.34 in
gst_vaapi_surface_new.

> 
> @@ +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)
Will fix this.
> 
> ::: 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.
Will fix this.

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