[Bug 790752] msdk: supports bufferpool

[GStreamer (GNOME Bugzilla)]

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

--- Comment #82 from Hyunjun Ko <zzoon at igalia.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #74)
> Review of attachment 368113 [details] [review]:
> 
> I would like a function to set the video info to the allocator, so it we
> could change the resolution of the allocator without reinstantiating it.

Okay, I'll add this to my to-do list.

> 
> ::: sys/msdk/gstmsdkbufferpool.c
> @@ +95,3 @@
> +      && (g_strcmp0 (allocator->mem_type, GST_MSDK_SYSTEM_MEMORY_NAME) != 0
> +          && g_strcmp0 (allocator->mem_type,
> +              GST_MSDK_VIDEO_MEMORY_NAME) != 0)) {
> 
> I've just learned that, as mem_type points to the defined string macro, it
> more efficient to use g_str_equal. But it is still safe use g_strcmp0. (I
> will change it for gstreamer-vaapi)
> 
> @@ +124,3 @@
> +      allocator =
> +          gst_msdk_video_allocator_new (priv->context, &video_info,
> +          priv->alloc_response);
> 
> what if the API use pases NULL to this parameters in buffer_pool_new()??
> isn't better to fail on set_config() requesting for a real allocator?

Good point. I'm working on this.

> 
> ::: sys/msdk/gstmsdksystemmemory.c
> @@ +137,3 @@
> +      &msdk_system_allocator->image_info);
> +
> +  memcpy (&surface->Info, &frame_info, sizeof (mfxFrameInfo));
> 
> is there need of this memcpy??? just fill the structure directly, isn't?

right!

> 
> @@ +215,3 @@
> +    GstAllocationParams * params)
> +{
> +  return gst_msdk_system_memory_new (allocator);
> 
> you need to pass @size to gst_msdk_system_memory_new() in order to use it in
> gst_memory_init(), not the video info size, if the request size is not
> enough for the frame it is better to inform the user.

Okay.

> 
> ::: sys/msdk/gstmsdkvideomemory.c
> @@ +74,3 @@
> +      &msdk_video_allocator->image_info);
> +
> +  memcpy (&surface->Info, &frame_info, sizeof (mfxFrameInfo));
> 
> ditto  (no need for memcopy imo)

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