GNOME Bugzilla – Bug 677516
[PATCH] v4l2src: ignore double framerates
Last modified: 2013-08-03 15:43:27 UTC
Created attachment 215696 [details] [review] [PATCH 0.10.31] v4l2src: ignore double framerates Hi, We (Fedora) have received bugreports from multiple users that cheese is not working on thinkpad x121e models. Since a friend of mine has such a model I booted a Fedora livecd there and managed to reproduce the issue. The problem is that gstreamer hangs deep inside the gstreamer core when doing caps negotation, this seems to be triggered by the /dev/video# node for the camera reporting the same frameinterval 4 times. The underlying cause of the reporting the same interval 4 times, is the USB descriptors of the camera reporting it can do 27.5xxx fps 4 times with 4 different frame interval values each differing only 0.1 microseconds from each other. The uvc kernel driver then translates this to the device having 4 framerates of each 27.5 fps, and this confuses gstreamer. I've not debugged the actual hang in the capabilities negotiation code, instead I've opted to modify the v4l2src to detect a device reporting the same framerate multiple times and to ignore the double reported rates. I've tested this and it fixes the problem on the x121e laptop I've access too. A patch for this is attached. Regards, Hans
Any chance you could get us a $ GST_DEBUG=*:5 cheese 2>dbg.log debug log from the hang? So we can track down the other bug - it really sounds like something we should fix as well.
(In reply to comment #1) > Any chance you could get us a > > $ GST_DEBUG=*:5 cheese 2>dbg.log > > debug log from the hang? So we can track down the other bug - it really sounds > like something we should fix as well. Hi, I no longer have access to the machine in question, but while developing the fix I've developed a little libv4l hack which allows to reproduce the problem. I'll attach the hack as hack.patch which modifies libv4l to trigger this with any uvc cam which supports > 1 framerate. This requires the v4l2src to be compiled with libv4l support of course. I've done 2 runs with a patched libv4l to reproduce, one with an unpatched v4l2src, I left cheese hanging for a couple of seconds before killing it: http://people.fedoraproject.org/~jwrdegoede/log.bad.gz And one with a v4l2src patched with the patch attached to this bug, where cheese ran successfully: http://people.fedoraproject.org/~jwrdegoede/log.good.gz I've done both runs under gdb and when I killed cheese in the bad scenario I did a bt, I'll attach a bt.log with the full bt. Regards, Hans
Created attachment 215735 [details] [review] Patch for libv4l to allow triggering the issue with any uvc cam with > 1 framerate p.s. To be clear: I did test my attached fix/patch on the real hardware. I did the quick hack to try an reproduce on another machine to confirm I was heading in the right direction when debugging the issue seen on the x121e.
Created attachment 215736 [details] backtrace from a hanging cheese (libv4l patched to trigger the issue, uvc cam, unpatched v4l2src)
I believe this is a duplicate of bug #684981 . Could you re-test if you still get the hang with GStreamer >= 1.0.1 and your patched libv4l ?
(In reply to comment #5) > I believe this is a duplicate of bug #684981 . That sounds like it is more or less the same though. Note that the fixes are different though, the fix for bug #684981 teaches the caps intersection code to deal with duplicate caps, where as my patch simply avoids creating duplicate caps for v4l2src-s, So I believe that my patch should still be applied, since reporting duplicate caps just is not good in generall. > Could you re-test if you still > get the hang with GStreamer >= 1.0.1 and your patched libv4l ? I'm afraid I'm completely swamped atm, so I don't have time for this, if you want you can try yourself by using a uvc camera which reports more then 1 framerate for a resolution + using my hack patch (which I'm happy to see I've attached here, otherwise it would have been long lost).
So since I was working on cheese again today, I decided to try and reproduce this. And indeed the original problem (the hang in the caps negotiation) is gone. But as said before, we still have v4l2src reporting duplicate caps, which I still consider undesriable, ie, with a patches libv4l2 to emulate a buggy cam on my non buggy cam, I get (small part of the total caps): video/x-raw, format=(string)YUY2, width=(int)800, height=(int)600, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1, framerate=(fraction){ 25/1, 25/1, 15/1, 10/1, 5/1 } Which simply does not look right. If you agree I can rebase my original patch, if you're not interested in filtering out the double framerates some buggy v4l2 devices report, feel free to close this.
I agree that it's not really right, but I'm not really keen on including code to work around this unless it actually causes problems. Maybe the probed result should simply be run through gst_caps_simplify() ? (does it detect duplicates in lists?) Sorry for being so difficult about this :)
(In reply to comment #8) > Maybe the probed result > should simply be run through gst_caps_simplify() ? I would do that (that's actually what I did in my experimental v4l2videodec branch).
(In reply to comment #8) > Maybe the probed result should simply be run through gst_caps_simplify() ? Yes I believe that would be a good idea regardless, and a good solution (note I've not actually tested this). So lets do that and then close this bug.
The reason we don't just gst_caps_simplify() the result is that _simplify() doesn't preserve the original order, which is meaningful in our case.
(In reply to comment #11) > The reason we don't just gst_caps_simplify() the result is that _simplify() > doesn't preserve the original order, which is meaningful in our case. Hmm, can you explain how the order is meaningful ? Note, not trying to be a smart-ass, just generally curious.
> Hmm, can you explain how the order is meaningful ? If there are multiple structures in caps (or lists), the first ones are assumed to be preferred over the latter. That's why try to put "non-emulated" formats in front of "emulated" ones. (There's some code to rank formats too, which is probably a bit dubious). In any case, I think we should close this unless it has catastrophic consequences if we don't work around it. I think it should be fixed in the driver or v4l instead.