GNOME Bugzilla – Bug 733616
v4l2object: code cleanup
Last modified: 2014-07-28 12:59:38 UTC
Currently, gst_v4l2_object_propose_allocation() always propose a pool with default min-buffers and max-buffers. But configured v4l2 bufferpool could have different values, for instance when v4l2 device can't allocate, max-buffers is reduced to min-buffers. I also attached patch with some fix for debug messages and minor refactoring.
Created attachment 281492 [details] [review] v4l2: set debug messages according to device type and IO mode
Created attachment 281493 [details] [review] v4l2sink: use directly 'obj' instead of 'v4l2sink->v4l2object'
Created attachment 281494 [details] [review] v4l2object: get current bufferpool min and max in propose_allocation() The interesting one :)
Review of attachment 281492 [details] [review]: Good catch, will merge soon.
Review of attachment 281493 [details] [review]: Cosmetic, but I'm fine with that.
Review of attachment 281494 [details] [review]: No, that isn't right. Propose allocation shall return it's minimum, regardless if a pool was configured or not. It's upstream element role to deactivate the pool if he want to renegotiate it. The only thing missing here is code that read the CID MIN_BUFFERS_FOR_INPUT, and upgrade the minimum from that (basically min = MAX(MIN_BUFFERS_FOR_INPUT, GST_V4L2_MIN_BUFFERS). See BaseSrc code of an example of how to re-negotiate a buffer pool (though even there, I think it should be improved a bit).
(In reply to comment #6) > Review of attachment 281494 [details] [review]: > > No, that isn't right. Propose allocation shall return it's minimum, regardless > if a pool was configured or not. It makes sense. > The only thing missing here is code that > read the CID MIN_BUFFERS_FOR_INPUT, and upgrade the minimum from that > (basically min = MAX(MIN_BUFFERS_FOR_INPUT, GST_V4L2_MIN_BUFFERS). I could implement this part if you want > See BaseSrc code of an example of how to re-negotiate a buffer pool Thanks for the reference.
(In reply to comment #7) > > The only thing missing here is code that > > read the CID MIN_BUFFERS_FOR_INPUT, and upgrade the minimum from that > > (basically min = MAX(MIN_BUFFERS_FOR_INPUT, GST_V4L2_MIN_BUFFERS). > I could implement this part if you want Sure, but should have it's own bug probably. I didn't have any drivers that implemented that yet. I wonder is ayaka, in it's encoder branch have that patch already. Btw, thanks for looking into this, it's all a bit tricky to get allocation negotiated and not blindly set.
Comment on attachment 281492 [details] [review] v4l2: set debug messages according to device type and IO mode commit 57ae11ac6f5c6821d265cb240aeb21aa056a8041 Author: Aurélien Zanelli <aurelien.zanelli@parrot.com> Date: Wed Jul 23 18:39:50 2014 +0200 v4l2: set debug messages according to device type and IO mode https://bugzilla.gnome.org/show_bug.cgi?id=733616
Comment on attachment 281493 [details] [review] v4l2sink: use directly 'obj' instead of 'v4l2sink->v4l2object' commit de799f4d8435d579f8c8ecc1aec6852589a5a6d4 Author: Aurélien Zanelli <aurelien.zanelli@parrot.com> Date: Wed Jul 23 18:40:10 2014 +0200 v4l2sink: use directly 'obj' instead of 'v4l2sink->v4l2object' https://bugzilla.gnome.org/show_bug.cgi?id=733616
I'm a bit puzzled what to do with this bug now, as the fix for the actual issue will be in OMX. Do you mind if I rename it to cleanup/varia and mark as closed ?
(In reply to comment #11) > I'm a bit puzzled what to do with this bug now, as the fix for the actual issue > will be in OMX. Do you mind if I rename it to cleanup/varia and mark as closed > ? It's ok for me.