[Bug 790752] msdk: supports bufferpool

[GStreamer (GNOME Bugzilla)]

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

--- Comment #95 from Hyunjun Ko <zzoon at igalia.com> ---
(In reply to sreerenj from comment #92)
> Review of attachment 368115 [details] [review]:
> 
> ::: sys/msdk/gstmsdkdec.c
> @@ +110,3 @@
> +        frame->output_buffer = NULL;
> +        goto retry;
> +       * We should be keep these buffers and check if it's unlocked.
> 
> I remember you were talking about the possibility to keep a separate surface
> pool and writing our own acquire_buffer() for the bufferpool. How about just
> overriding the acquire_buffer() as:
> 
> gst_msdk_buffer_pool_acqurre_buffer{
>   buffer = GST_BUFFER_POOL_CLASS
>       (gst_msdk_buffer_pool_parent_class)->acquire_buffer()
>   if (buffer.surface.Data.locked)
>       release_buffer() and try again.
>   else
>        ret buffer;
> }
> 

That's exactly what i was thinking about. and that's why I was confused since
you said it would be complicated.
I'm planning to implement it after these patches are merged.


> @@ +731,3 @@
> +    return NULL;
> +  }
> +
> 
> I think we should move/refactor this function as a common utility method for
> both encode and decoder.
> It is almost the same in both, you may just need to pass few more parameters.
> 

Right. There's something more to refactor also. (eg. GstMsdkSurface in
encoder/decoder)

> @@ +818,3 @@
> +
> +    memcpy (&thiz->output_info, &info_from_caps, sizeof (GstVideoInfo));
> +    memcpy (&info_aligned, &info_from_caps, sizeof (info_aligned));
> 
> Just assign the struct, no need of memcpy.

ok though this is not from my patch.

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