[Bug 790752] msdk: supports bufferpool

[GStreamer (GNOME Bugzilla)]

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

--- Comment #84 from Hyunjun Ko <zzoon at igalia.com> ---
(In reply to sreerenj from comment #77)
> Review of attachment 368112 [details] [review]:
> 
> ::: sys/msdk/gstmsdkallocator_libva.c
> @@ +65,3 @@
> +              MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET)))
> +    return MFX_ERR_UNSUPPORTED;
> +
> 
> I am bit confused here. Could you please add some comments?
> 
> 1: Consider the pipeline: "videotestsrc ! msdkh264enc ", is it supposed to
> fail (in won't in practice)? I am wondering why msdk has added
> MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET into the req->type even if there is
> no decoder in the pipeline! 

I'm not sure either, to be honest. :) 
But I guess MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET is for both encoder and
decoder, MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET is for VPP, otherwise
MFX_MEMTYPE_SYSTEM_MEMORY.

Regarding this, the document says that "The VA API does not define any surface
types and the application can use either
MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET or
MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET to indicate data in video memory."

I'll comment this out.


> 
> 2: According to my testing there is too much memory (number of surfaces)
> allocation for a simple transcode pipeline (mfxh264dec async-depth=1 !
> mfxh264enc async-depth=1).
> You can test with the video.mp4 which I have sent recently :), It is
> allocating 20+ surfaces (without counting the internal allocation)
> 

That's normal if using video memory especially for h264/5 decoding (not
encoding). I don't know why the driver requires that many surfaces.


> @@ +106,3 @@
> +     *
> +     * See https://github.com/Intel-Media-SDK/samples/issues/13 for more
> details.
> +     */
> 
> Ya, this is so nasty, but we have no option other than doing the same!
> The OpenSource msdk may fix this soon, but we can't remove this(unless we
> put a tight dependency on the next msdk opensource release)
> because of backward compatibility and all most all the customers are
> currently using Closed source version of msdk I believe.
> 
> BTW, MSDK developers saying that it requires for all encoders, but I am not
> sure :)
> 
> @@ +195,3 @@
> +
> +  if (mem_id->fourcc != MFX_FOURCC_P8) {
> +    va_status = vaDeriveImage (dpy, *va_surface, &mem_id->image);
> 
> I am still not 100% sure whether is safe to do it like this. What happens if
> msdk call frame_lock on a surface
> which we already mapped by a GStreamer element? But we haven't seen any case
> like that and hopefully, msdk never
> call this except for the internal frames.

That's also what I'm worried about though I've never seen it's called from the
driver for external frames. Let's see what's happening here.

> 
> For now, I am fine to keep this code as it is since our main target is to
> get the code in before upstream feature freeze :)

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