[Bug 781537] NVDEC - Nvidia Decoder plugin

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #352853|none                        |reviewed
             status|                            |

--- Comment #41 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 352853:
 --> (https://bugzilla.gnome.org/review?bug=781537&attachment=352853)

Just some initial comments below

::: configure.ac
@@ +1920,3 @@
+  save_CPPFLAGS="$CPPFLAGS"
+  CPPFLAGS="$NVCUVID_CFLAGS $save_CPPFLAGS"
+  AC_CHECK_HEADER([nvcuvid.h], [HAVE_NVCUVID_H=yes],

Why don't you use pkg-config?

::: sys/nvdec/gstnvdec.c
@@ +120,3 @@
+  CUgraphicsResource *resources;
+  guint num_resources;
+} GstNvDecCudaGraphicsResourcesMeta;

Why is this a GstMeta instead of a custom GstMemory (e.g. based on
GstGLBaseMemory)?

@@ +617,3 @@
+  else if (!g_strcmp0 (caps_name, "video/x-h265"))
+    parser_params.CodecType = cudaVideoCodec_HEVC;
+  else {

Add some {} above here

@@ +735,3 @@
+    pending_frame = pending_frames->data;
+    if (!pending_frame->system_frame_number)
+      break;

Why?

@@ +790,3 @@
+        decode_params = (CUVIDPICPARAMS *) item->data;
+        pending_frame = pending_frames->data;
+        pending_frame->system_frame_number = decode_params->CurrPicIdx + 1;

You should never override the system_frame_number, it's read-only from a
subclass point of view

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