GNOME Bugzilla – Bug 622500
[v4l2sink] destroy open buffers when changing to NULL
Last modified: 2010-07-06 14:03:57 UTC
Created attachment 164405 [details] [review] [gstv4l2sink] destroy open buffers when changing to NULL In the case we change the State from READY_TO_NULL the open buffers still hold a open dup filedescriptor to the device, therefore the device release function will not be called and the device probably answer with -EBUSY when we reopen it at transition NULL_TO_READY.
fyi, this is also solved by one of the patches on #612244 I'll refresh those patches over the weekend of before.. but maybe would be a good idea to get them merged. I'm not sure if this ticket should be marked as a duplicate?
Some of those patches in the other bug look mergeable, others look rather dubious or make me think v4l2sink should be moved to -bad really. I might to apply some of these patches after the first pre-release, but I don't really want to block on this now. I'm not merging this patch because it doesn't conform to our indenting guidelines (please don't disable the git pre-commit hook and/or install gnu-indent), and I can't be bothered to clean it up now. And the patch from the other bug is missing the gst_v4l2_object_stop_streaming() bit and also contains a gratuitious(?) STATE_OFF => STATE_PENDING_STREAMON change.
Due to lack of response from original reporter I've only committed a minimal change now, ie. the buffer_pool_destroy(), and IIRC Rob expressed doubts about the stop_streaming bit. I assume the STATE_OFF => STATE_PENDING_STREAMON change in the other state transition (in the other patch) wasn't really related or required.
Sorry, i have been busy last days on hacking the kernel. I think the not pretty indented "gst_v4l2_object_stop_streaming(v4l2sink->v4l2object);" was just added as mentioned in the TODO comment above. At this moment i don't have lot time to check on this again. But it seems to me that the applied patch should still release the locked filedescriptor in the bufferpool. So, thanks a lot. mgr
Ok, thanks for the follow-up.
(In reply to comment #3) > > I assume the STATE_OFF => STATE_PENDING_STREAMON change in the other state > transition (in the other patch) wasn't really related or required. Just fwiw, the STATE_OFF => STATE_PENDING_STREAMON is to properly handle state change sequence like: PAUSED->READY->PAUSED.. In this case, the buffers are still allocated, but a VIDIOC_STREAMON is needed in order to start playing again. so maybe not exactly the same issue as the need to destroy buffers when going back to NULL state... but a related state change issue. The _stop_streaming() change should definitely be a different patch... because I think a few other things are needed to handle renegotiating caps properly. (It's something on my TODO list, but haven't gotten too yet.. feel free to file a bug and assign to me for proper handling of caps renegotiation.)