[Bug 790752] msdk: supports bufferpool

[GStreamer (GNOME Bugzilla)]

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

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

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

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

::: sys/msdk/gstmsdkbufferpool.c
@@ +145,3 @@
+  GstMsdkBufferPool *pool;
+
+  pool = g_object_new (GST_TYPE_MSDK_BUFFER_POOL, NULL);

a simpler version of this is

return g_object_new (GST_TYPE_MSDK_BUFFER_POOL, NULL);

::: sys/msdk/gstmsdkmemory.c
@@ +141,3 @@
+{
+  if (!ensure_surface (mem))
+    goto error_no_surface;

since it is not expected to have a complex implementation, there is no need to
handle this single error with the goto pattern.

@@ +155,3 @@
+gst_msdk_allocator_create_surface (GstAllocator * allocator)
+{
+  mfxFrameInfo frame_info;

as this is a complete structure in the stack, you can initialized here by = {
0, } or something similar, avoiding the memset done afterwards. Thus, it is
done at compilation time and not at runtime...

@@ +175,3 @@
+gst_msdk_memory_new (GstAllocator * base_allocator)
+{
+  GstMsdkAllocator *const allocator = GST_MSDK_ALLOCATOR_CAST
(base_allocator);

I would do this cast after the g_return_val_if_fail(), just for clarity

@@ +232,3 @@
+  g_slice_free (mfxFrameSurface1, mem->surface);
+  mem->cache = NULL;
+  mem->surface = NULL;

have you have looked g_clear_pointer ()  ?

@@ +274,3 @@
+    return NULL;
+
+  allocator->image_info = *image_info;

what about, instead of storing the gstvideoinfo, you store the mfxFrameInfo
directy?

I mean, receive as parameter GstVideoInfo, but convert it and store it in the
allocator structure as mfxFrameInfo, then you don't need to execute the
conversion every time a surface is allocated.

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