race in camerabin2 when starting a video recording shortly after stopping another one

[Raluca-Elena Podiuc <ralucaelena1985 gmail com>]

Hi,

I've been working on porting cheese to camerabin2. It works pretty
well right now, but there still are some quirks that I'll try to
address.

There is a race when I start a video recording very fast after
stopping one before it.


= Functions involved =

When starting/stopping a video recording there are a few steps that happen:

cheese-camera issues 'start-capture' signal on camerabin2
  https://github.com/raluca-elena/cheese/blob/camerabin2/libcheese/cheese-camera.c#L791

camerabin2 passes 'start-capture' signal to wrappercamerasrc
  https://github.com/raluca-elena/gst-plugins-bad/blob/master/gst/camerabin2/gstcamerabin2.c#L268

wrappercamerasrc derives from basecamerasrc.

basecamerasrc handles 'start-capture' in gst_base_camera_src_start_capture:
 https://github.com/raluca-elena/gst-plugins-bad/blob/master/gst-libs/gst/basecamerabinsrc/gstbasecamerasrc.c#L226

and calls ->start_capture which wrappercamerasrc sets to
gst_wrapper_camera_bin_src_start_capture:
  https://github.com/raluca-elena/gst-plugins-bad/blob/master/gst/camerabin2/gstwrappercamerabinsrc.c#L971


Similar for stop-capture.



gst_base_camera_src_start_capture also sets an internal boolean
'src->capturing' to true/false and calls
    g_object_notify (G_OBJECT (src), "ready-for-capture");
to notify users of the change.

camerabin2 listens for this change in gst_camera_bin_src_notify_readyforcapture
 https://github.com/raluca-elena/gst-plugins-bad/blob/master/gst/camerabin2/gstcamerabin2.c#L328
 where, if it's ready to capture, it puts some elements to NULL,
configure them and sets them to PLAYING


Lastly there's the video probe on one of wrappercamerabinsrc's
video-src output pad: gst_wrapper_camera_bin_src_vidsrc_probe
  https://github.com/raluca-elena/gst-plugins-bad/blob/master/gst/camerabin2/gstwrappercamerabinsrc.c#L236



= State machine =

wrappercamerabinsrc's video_rec_status is changed in 3 places:
1. gst_wrapper_camera_bin_src_vidsrc_probe:
  for each new buffer on video-src:
  switch (video_rec_status):
    DONE: drop buffer (
    STARTING: video_rec_status=RUNNING
    RUNNING: let buffer pass to encodebin
    FINISHING: send EOS
                       video_rec_status=DONE
                       src->capturing=false
                       notify::ready-for-capture


2. gst_base_camera_src_start_capture:
  gst_wrapper_camera_bin_src_start_capture:
     if (video_rec_status == DONE) video_rec_status=STARTING
  notify::ready-for-capture
     gst_camera_bin_src_notify_readyforcapture
        puts elements to PLAYING which drives buffers through
video-src calling gst_wrapper_camera_bin_src_vidsrc_probe


3. gst_base_camera_src_stop_capture
  gst_wrapper_camera_bin_src_stop_capture:
    switch (video_rec_status)
      STARTING: video_rec_status=DONE (abort)
       RUNNING: video_rec_status=FINISHING
       default: do nothing


Summary:
 - start-capture: if status==DONE => start=STARTING
 - vidsrc_probe: state machine
 - stop-capture: if status==RUNNING => start=FINISHING

vidsrc_probe runs from a gstreamer-thread, start/stop capture run from
gtk-threads (callbacks from pushed buttons)


Problem: if we do 'stop' and 'start' very fast one after the other we
could have start-capture running before vidsrc_probe managed to update
the state machine from FINISHING to DONE. When the start runs it finds
the state=FINISHING not STARTING/RUNNING and does nothing.

I encounter this in Cheese when I push "start/stop recording video"
very rapidly.

This commit: https://github.com/raluca-elena/gst-plugins-bad/commit/8402cfb253c0522c8f9ee4a68e9ead61dc48ff62
seems to make things a bit better. It delays the
notify::ready-for-capture signal after ->start_capture. The
Video-encodebin is put in PLAYING after we do start_capture and thus
the vidsrc_probe (state machine) runs after start_capture had a chance
to run first (state machine initialization).


I'm still working on this (have been for the last few days), but if
you have some insight on how this can be fixed simpler I'm all ears :)



-- 
Raluca-Elena