[Bug 793413] msdk: manage mfxFrameSurfaces seperately with other surfacepool.

[GStreamer (GNOME Bugzilla)]

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

--- Comment #11 from sreerenj <bsreerenj at gmail.com> ---
(In reply to Hyunjun Ko from comment #10)
> (In reply to sreerenj from comment #9)
> > (In reply to Hyunjun Ko from comment #8)
> > > (In reply to sreerenj from comment #7)
> > > > 
> > > > Assume it is a decoder.
> > > > There are many possible cases:
> > > > 
> > > > a)the downstream using 4 buffers, (it would be too strange if downstream
> > > > keep four buffers without releasing, but possible, maybe using queue before
> > > > each postprocessing element)
> > > > 
> > > > b) Decoder is using those 4 buffers (then the "Locked" flag here should be 1
> > > > too because the surfaces are being used by MediaSDK too)
> > > > 
> > > > 
> > > > Currently, gst_msdk_video_memory_replace_surface() will try to replace a
> > > > surface associated with the buffer if the acquire_buffer() returns a buffer
> > > > which has locked flag set. 
> > > > How can we guarantee that the surface-X (the new unlocked surface which
> > > > supposed to replace the associated surface in acquired_buffer) is not
> > > > required by dowstream anymore? Here the Msdk might have decreased the Lock
> > > > count of Surface-X, but how can we ensure that the decoder already pushed
> > > > this surface-X 
> > > > downstream? Is it possible to have this surface-X been in a locked state for
> > > > referencing and now decreased the lock count, but not yet pushed downstream?
> > > 
> > > Ah... that's a good point, IIUC.
> > > 
> > > To handle that, there should be 3 lists (avail, used, locked?) and then
> > > 
> > > - The used list : surfaces associated with the gst buffer acquired.
> > > - The locked list : when releasing the buffer, if surface is still locked,
> > > we put it to this list.
> > > - The available list: the surfaces get unlocked in the locked list should
> > > move to this list or surfaces that are never used.
> > > 
> > > we can search only the locked list without the used list when we need to
> > > replace.
> > > 
> > > What do you think?
> > 
> > 
> > I'm still wondering what is the issue with the approach we mentioned in
> > comment-3 and comment-4.
> > 
> 
> Gstbufferpool doesn't know if a buffer is in use in the driver as you know.
> If there are 5 allocated buffers, 4 in use, 1 in release (but still being
> used in the driver)  the pool always gives the buffer to the user when
> calling acquire_buffer.
> 

"4 in use": means you are not supposed to replace the surface in any of those 4
buffers. Those are already acquired by either decoder or downstream. So our
only choice is to check whether the last one available buffer.
In this case, the infinite loop can be avoided by trying only a specific number
of times (eg: try till the count reach the size of gstbufferpool).

The point is that "release_buffer" will always release buffer back to pool
irrespective of whether MSDK keeping the lock for referencing. 

Am I missing something?


> If we implement something you're thinking in acquire_buffer, it causes
> infinite loop.
> 
> And also see the comment 6.

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