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 621723 - v4l2src crashes on PLAYING->READY->PLAYING
v4l2src crashes on PLAYING->READY->PLAYING
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.25
Other Linux
: Normal normal
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-06-16 03:34 UTC by Havoc Pennington
Modified: 2010-06-16 15:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
v4l2src: in negotiate, check for error return from set_caps (1.46 KB, patch)
2010-06-16 15:25 UTC, Havoc Pennington
committed Details | Review
v4l2src: do not try to change device format if it's already correct (1.76 KB, patch)
2010-06-16 15:25 UTC, Havoc Pennington
committed Details | Review

Description Havoc Pennington 2010-06-16 03:34:30 UTC
I believe the issue is that in gst_v4l2src_set_caps(), if set_capture fails then we do not capture_init(), which would normally set the buffer pool non-NULL. Then trying to use the buffer pool goes boom.

if (!gst_v4l2src_set_capture (v4l2src, format->pixelformat, w, h, fps_n,
          fps_d))
    /* error already posted */
    return FALSE;

  if (!gst_v4l2src_capture_init (v4l2src, caps))
    return FALSE;

It looks like this is still a problem in git though I saw the bug in 0.10.25

The set_capture fails due to:
libv4l2: error setting pixformat: Device or resource busy

gst_v4l2src_negotiate() does not check the return value of set_caps and so the negotiation succeeds even though set_caps failed.


  • #0 gst_v4l2_buffer_pool_dqbuf
    at gstv4l2bufferpool.c line 509
  • #1 gst_v4l2src_grab_frame
    at v4l2src_calls.c line 128
  • #2 gst_v4l2src_get_mmap
    at gstv4l2src.c line 864
  • #3 gst_v4l2src_create
    at gstv4l2src.c line 904
  • #4 gst_push_src_create
    at gstpushsrc.c line 117
  • #5 gst_base_src_get_range
    at gstbasesrc.c line 1971
  • #6 gst_base_src_loop
    at gstbasesrc.c line 2215
  • #7 gst_task_func
    at gsttask.c line 234
  • #8 default_func
    at gsttaskpool.c line 70
  • #9 g_thread_pool_thread_proxy
    at gthreadpool.c line 315
  • #10 g_thread_create_proxy
    at gthread.c line 1898
  • #11 start_thread
    at pthread_create.c line 300
  • #12 clone
    at ../sysdeps/unix/sysv/linux/i386/clone.S line 130

Comment 1 Havoc Pennington 2010-06-16 15:25:26 UTC
Created attachment 163837 [details] [review]
v4l2src: in negotiate, check for error return from set_caps

Fixes #621723  (partially)

set_caps can fail if the video device is running, in that case
setting its format leads to EBUSY.

If set_caps fails then we will not have set up the buffer pool
(it will be NULL) which leads to a crash when we try to pull
buffers. If we fail the negotiate on set_caps failure, then we
won't go to playing state and won't crash.

This is a small improvement. Of course, a nicer fix would
be to make set_caps work in the case where the format is
unchanged. If the format has changed, failing is
probably correct because we need to close the device
(go to NULL state) in order to set caps.
Comment 2 Havoc Pennington 2010-06-16 15:25:29 UTC
Created attachment 163838 [details] [review]
v4l2src: do not try to change device format if it's already correct

This allows set_caps to succeed if caps change in a way that
would not modify the format we're getting from the hardware.
Otherwise if not in NULL state, setting caps would fail
with EBUSY.

With this change, in some cases it's OK to go PLAYING->READY->PLAYING
rather than PLAYING->NULL->PLAYING to avoid a time-consuming close
and reopen of the device.

Fixes #621723
Comment 3 Sebastian Dröge (slomo) 2010-06-16 15:49:03 UTC
commit f06b105058e73162275b50bdba89c6791e218d03
Author: Havoc Pennington <hp@pobox.com>
Date:   Wed Jun 16 11:21:35 2010 -0400

    v4l2src: do not try to change device format if it's already correct
    
    This allows set_caps to succeed if caps change in a way that
    would not modify the format we're getting from the hardware.
    Otherwise if not in NULL state, setting caps would fail
    with EBUSY.
    
    With this change, in some cases it's OK to go PLAYING->READY->PLAYING
    rather than PLAYING->NULL->PLAYING to avoid a time-consuming close
    and reopen of the device.
    
    Fixes #621723

commit 9b9f9d0a2a93365e2577c5bad8591f82bbf9e6d0
Author: Havoc Pennington <hp@pobox.com>
Date:   Wed Jun 16 11:09:17 2010 -0400

    v4l2src: in negotiate, check for error return from set_caps
    
    Fixes #621723  (partially)