[Bug 796985] kmssink modesetting fails for multi-planar buffers

[GStreamer (GNOME Bugzilla)]

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

--- Comment #7 from Devarsh Thakkar <devarsht at xilinx.com> ---
Review of attachment 373369:
 --> (https://bugzilla.gnome.org/review?bug=796985&attachment=373369)

Thanks for the patch Philip.
Only suggestion I have is to directly use GST_VIDEO_INFO_HEIGHT and
GST_VIDEO_INFO_WIDTH instead of using local declarations.
Please refer in-code comments for reference.

Regards,
-Devarsh

::: sys/kms/gstkmssink.c
@@ +420,3 @@
   GstKMSMemory *kmsmem;
+  guint32 w;
+  guint32 h;

In my opinion, above declarations can be avoided, please see my next comment.

@@ +445,2 @@
   for (i = 0; i < conn->count_modes; i++) {
+    if (conn->modes[i].vdisplay == h && conn->modes[i].hdisplay == w) {

I think we can avoid local integer declarations for width and height and
directly use GST macros as shown below :

if (conn->modes[i].vdisplay == GST_VIDEO_INFO_HEIGHT (vinfo); &&
conn->modes[i].hdisplay == GST_VIDEO_INFO_WIDTH (vinfo);)

--- Comment #8 from Devarsh Thakkar <devarsht at xilinx.com> ---
Review of attachment 373369:
 --> (https://bugzilla.gnome.org/review?bug=796985&attachment=373369)

Thanks for the patch Philip.
Only suggestion I have is to directly use GST_VIDEO_INFO_HEIGHT and
GST_VIDEO_INFO_WIDTH instead of using local declarations.
Please refer in-code comments for reference.

Regards,
-Devarsh

::: sys/kms/gstkmssink.c
@@ +420,3 @@
   GstKMSMemory *kmsmem;
+  guint32 w;
+  guint32 h;

In my opinion, above declarations can be avoided, please see my next comment.

@@ +445,2 @@
   for (i = 0; i < conn->count_modes; i++) {
+    if (conn->modes[i].vdisplay == h && conn->modes[i].hdisplay == w) {

I think we can avoid local integer declarations for width and height and
directly use GST macros as shown below :

if (conn->modes[i].vdisplay == GST_VIDEO_INFO_HEIGHT (vinfo); &&
conn->modes[i].hdisplay == GST_VIDEO_INFO_WIDTH (vinfo);)

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