v4l2src regression in git master

[Nicolas Dufresne]

First step of the fix:
https://bugzilla.gnome.org/show_bug.cgi?id=744573

Le 2015-02-14 14:34, Nicolas Dufresne a écrit :
> Hi Everyone,
>
> Sebastien mention to me last week that the state of v4l2src was not 
> good in git master. I did some investigation while on a plane 
> yesterday. What I notice have changed is the following (note that I'll 
> file bugs about each of these soon, I just wanted to group all these 
> issues together as the link might not be evident) :
>
> Decodebin systematically cause a reconfigure after first frame. This 
> cause GstBaseSrc to re-run allocation query. This is not really a bug, 
> but in fact the effect of having another bug fixed. During the second 
> pool configuration, the pool is already active. This would fail in the 
> past (but we were ignoring return value, making it work by accident 
> most of the time). In recent pool, we are a bit more tolerant, in the 
> sense that succeed the configuration if the pool is active and the 
> configuration have not changed. We do this using equality on the 
> structure, which is too strict since adjustment are made by the pool. 
> The pool also do one more step in trying to succeed setting the new 
> configuration, which is to stop the pool. Note that at this point, the 
> allocation query has already run, hence the pipeline should have been 
> drained. Though, because the check is too strict, this lead to an 
> unfortunate reallocation of the buffers. On V4L2 side, this also mean 
> we restart streaming, which cause additional delay between the first 
> and second frame.
>
> The equality check is obviously too strict. I have two proposition to 
> solve this issue. The first solution is that when the pool is active, 
> and configuration does not match, I would simply return FALSE. The 
> caller will then get new configuration and validate it. If it's still 
> compatible (which is most likely), it will simply call again 
> set_config() with the configuration which is equal and succeed. In 
> that scenario, the pool will never try to stop itself in set_config(), 
> which removes the unwanted side effect and the set_config() 
> documentation is perfectly respected. This would also mean that 
> renegotiation of allocation does not happen unless the element support 
> it. Surprisingly, this is the old behaviour, since the new 
> configuration was never taken into account unless a new pool was 
> selected.
>
> The second solution would be to relax this check, checking for 
> compatible configuration. Though this check remains a grey zone. The 
> generic method that we added for that was not meant to be a perfect 
> compatibility, but just a convenience function that cover the most 
> common case. I'm starting to think it's not an ideal approach, but it 
> is something that could work too. This would allow somehow implicit 
> reconfiguration when the pool didn't changed.
>
> Now, this isn't the only thing that have changed. For reason I still 
> don't understand, the GstAdapter used in the jpegdec internal parser 
> started to randomly copy GstBuffers, which lead to calling 
> v4l2mem_share(). I found that the implementation in GstV4l2Allocator 
> is currently broken and lead to the memory to being unmmap() too early 
> hence lead to crash. This does not happen with raw format as they are 
> expected to be framed. This I'll fix in both master and 1.4, I have 
> prepared a patch already. There might be something we could add to the 
> caps to get this parsing step optimized, as we know v4l2 buffer are 
> framed (maybe jpegdec has no such optimization, I didn't check).
>
> I also notice new issues with camerabin but my flight was not long 
> enough, so this isn't investigated yet. We patched it for 1.4 to stop 
> trying to reconfigure v4l2src (which is still not supported). But it 
> seems that behaviour just came back. It is possibly related to the 
> reconfigure event going through now. I think the right solution here 
> would be to implement re-negotiation. I can't guaranty when this will 
> happen, neither if it will happen for 1.6. So we should probably patch 
> camerabin again if fixing the previous issues does not also fix this one.
>
> If you are noticing other regression in this element, please let me 
> know. We have enabled many kernel features that we found are badly 
> proxied by libv4l2. We also don't have (yet) any automated test for 
> this element. For that, kernel 3.19 now ship with vivid test drivers. 
> I am looking forward using this within gst-validate scenario in a near 
> future.
>
> cheers,
> Nicolas
>
> p.s. I'll reply to this when bugs URI are available (some bugs already 
> exist)
> _______________________________________________
> gstreamer-devel mailing list
> gstreamer-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel