After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 740636 - v4l2src: framerate is not always set on driver
v4l2src: framerate is not always set on driver
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.4.4
Other Linux
: Normal major
: 1.4.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-24 16:08 UTC by Chris Maes
Modified: 2015-03-31 14:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a proposed fix (1.14 KB, patch)
2014-11-24 16:08 UTC, Chris Maes
none Details | Review
[PATCH] v4l2object: Always set format (4.24 KB, patch)
2014-12-09 23:41 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Chris Maes 2014-11-24 16:08:22 UTC
Created attachment 291377 [details] [review]
a proposed fix

when launching a simple chain and imposing a certain framerate, for example in this chain:

gst-launch-1.0 -ef v4l2src device=/dev/video0 ! video/x-raw,format=GRAY8,framerate=60/1 ! mwebserver ! fakesink

(mwebserver is a personal plugin), then the framerate is never set in the driver. I dug in the code and suspect that this bug was introduced in commit a7286563ee1fb0d3d2b3c24952288a8e493875da from the gst-plugins-good repository. I put a patch for a quickfix in attachment, but cannot guarantee this is complete.

this code should be called but is never called: 

if (v4l2_ioctl (fd, VIDIOC_S_PARM, &streamparm) < 0)
      goto set_parm_failed;
Comment 1 Chris Maes 2014-11-24 16:08:54 UTC
the bug did not appear in gstreamer-1.0, but appears in gstreamer-1.4.
Comment 2 Nicolas Dufresne (ndufresne) 2014-12-01 18:36:49 UTC
Having this filed against GStreamer project instead of gstreamermm-plugins-good will help ;-P
Comment 3 Nicolas Dufresne (ndufresne) 2014-12-01 18:39:11 UTC
To be honest, I'm tempted to say we should probably not optimize out the S_FMT in the first place.
Comment 4 Nicolas Dufresne (ndufresne) 2014-12-09 23:40:45 UTC
The context is that until recently, we had terrible hack that would get/set
format while probing. This was actually leading toward format changing all the
time. Clearly that had drastic performance issue, since getting/setting format
thousands of times can take time. Though there is no measurable rational for
avoiding a single format set, and this code has been the source of bugs for a
long time. So I think we should just strip it up, and always set the format and
the framerate.
Comment 5 Nicolas Dufresne (ndufresne) 2014-12-09 23:41:59 UTC
Created attachment 292414 [details] [review]
[PATCH] v4l2object: Always set format


Right now we try to be clever by detecting if device format have
changed or not, and skip setting format in this case. This is valid
behaviour with V4L2, but it's also very error prone. The rational
for not setting these all the time is for speed, though I can't
measure any noticeable gain on any HW I own. Also, until recently,
we where doing get/set on the format for each format we where
probing, making it near to impossible that the format would match.
This also fixes bug where we where skipping frame-rate setting if
format didn't change.

https://bugzilla.gnome.org/show_bug.cgi?id=740636
---
 sys/v4l2/gstv4l2object.c | 70 +-----------------------------------------------
 1 file changed, 1 insertion(+), 69 deletions(-)
Comment 6 Nicolas Dufresne (ndufresne) 2014-12-09 23:43:07 UTC
Please let me know if that patch works as well for you.
Comment 7 Nicolas Dufresne (ndufresne) 2014-12-13 17:52:33 UTC
Chris, any comment ?
Comment 8 Chris Maes 2014-12-15 07:58:32 UTC
You patch looks good; a more thorough change than what I proposed. I don't have an environment here that uses the gstreamer-1.4 framework yet. (I tested the upgrade to 1.4 for a while using the packman-rpms, but then we abandoned that upgrade for a while...)
Comment 9 Nicolas Dufresne (ndufresne) 2014-12-16 00:01:57 UTC
Comment on attachment 292414 [details] [review]
[PATCH] v4l2object: Always set format

1.5
commit 3dae65ede86e904145db27001e8ff60b9db03a60
Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Date:   Tue Dec 9 15:09:56 2014 -0500

    v4l2object: Always set format
    
    Right now we try to be clever by detecting if device format have
    changed or not, and skip setting format in this case. This is valid
    behaviour with V4L2, but it's also very error prone. The rational
    for not setting these all the time is for speed, though I can't
    measure any noticeable gain on any HW I own. Also, until recently,
    we where doing get/set on the format for each format we where
    probing, making it near to impossible that the format would match.
    This also fixes bug where we where skipping frame-rate setting if
    format didn't change.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=740636

1.4 commit 67ac8
Comment 10 Chris Maes 2015-03-31 14:17:54 UTC
Nicolas, I upgraded and your fix works fine; just wanted to confirm :) tx.
Comment 11 Nicolas Dufresne (ndufresne) 2015-03-31 14:45:58 UTC
Thanks for reporting !