[Bug 733864] v4l2videodec: Implement EOS handling through V4L2_DEC_CMD_STOP

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=733864

--- Comment #26 from Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> ---
(In reply to Philipp Zabel from comment #25)
> I don't think I used the wrong lock. The GST_TASK_WAIT expands to
> g_cond_wait(GST_TASK_GET_COND (task, GST_OBJECT_GET_LOCK (task)), so
> g_cond_wait first tries to unlock the object lock, which is what protects
> task->running and task->state.

You are right, I miss-read that. The other lock is a recursive lock, so it
could not have been used for a GCond, sorry.

> 
> Also I think spinning on (task->running) won't terminate because
> gst_v4l2_video_dec_loop() only calls gst_pad_pause_task().
> gst_pad_task_stop() is called from gst_v4l2_video_dec_finish() right after
> this code.
> 
> How about this instead:
> 
>     GstTask *task = decoder->srcpad->task;
> 
>     /* If the decoder stop command succeeded, just wait until processing is
>      * finished */
>     GST_OBJECT_LOCK (task);
>     while (GST_TASK_STATE (task) == GST_TASK_STARTED)
>       GST_TASK_WAIT (task);
>     GST_OBJECT_UNLOCK (task);
> 
> Or should the beach: label in gst_v4l2_video_dec_loop() be changed to call
> gst_pad_task_stop() if ret == GST_FLOW_EOS?

I see, both ways sounds reasonable. With the second, we'd need to leave the
thread for both EOS and ERROR, but not flushing. In general though, it's
probably more efficient to keep that thread running as long as it's possibly
needed again (recoverable). That's probably why going to pause is what makes
sense. So I'd go for first option.

I think the task handling in general is very hairy in v4l2videodec and that
accessing the state like you suggest could help cleaning things up. I'll be
looking forward this method if you aren't faster then me ;-P

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.