GNOME Bugzilla – Bug 739472
multifilesrc: Lost the ability to start at a different frame by setting index property
Last modified: 2015-08-18 12:22:11 UTC
The index parameter seems to get ignored. This is gstreamer 1.4.2 from Fedora 21 pre-release $ gst-launch-1.0 qtmux name=mux ! filesink location=\"timelapse.mp4\" multifilesrc location=\"DSC%05d.JPG\" index=19 caps=\"image/jpeg,framerate=\(fraction\)30/1\" ! jpegdec ! videoconvert ! videoscale ! video/x-raw ! videorate ! x264enc bitrate=10000 ! mux. Setting pipeline to PAUSED ... Pipeline is PREROLLING ... ERROR: from element /GstPipeline:pipeline0/GstMultiFileSrc:multifilesrc0: Error while reading from file "DSC00000.JPG". Additional debug info: gstmultifilesrc.c(467): gst_multi_file_src_create (): /GstPipeline:pipeline0/GstMultiFileSrc:multifilesrc0: Failed to open file 'DSC00000.JPG': No such file or directory ERROR: pipeline doesn't want to preroll. Setting pipeline to NULL ... Freeing pipeline ...
I confirm,
Regression introduced when seeking support was added as basesrc does a seek at the beginning (right after start) to 0. So multifilesrc ignores the property value.
Note: using the 'start-index' property instead seems to work as a workaround.
Any sane way to fix this regression? The seek will always be done at the beginning and it will reset the start index to 0.
So was introduced by this: commit 1fc591238bd7018a93d16e977dfeaf5f68092059 Author: Thibault Saunier <thibault.saunier@collabora.com> Date: Wed Nov 13 20:18:17 2013 -0300 multifilesrc: Implement seeking in case of multiple images https://bugzilla.gnome.org/show_bug.cgi?id=712254 Maybe multifilesrc should just set the initial seek in basesrc based on the property value? The [0,-1] seek in basesrc IIRC does not happen if another seek (e.g. in READY) was received before.
Created attachment 301534 [details] [review] fix index property being ignored After a few false starts, here's patch doing what was suggested above. The seek to 0 is indeed overridden by the preliminary seek, if any is set.
Review of attachment 301534 [details] [review]: Not sure about this patch. Replacing the pending_seek on basesrc seems weird, but it is public- maybe someone else has an opinion?
Created attachment 307052 [details] [review] multifilesrc: index handling in initial seek Index property ignored because base src always do initial seek at the beginning. start-index and stop-index are the boundary conditions for index. Initial seek to 0 should always seek to start-index. If index property is mentioned before starting the pipeline, it should also update the start-index property. With this initial seek will set the index to start-index and index property wont be ignored. Please review the patch to handle the index property in initial seek.
Review of attachment 307052 [details] [review]: ::: gst/multifile/gstmultifilesrc.c @@ +303,3 @@ case PROP_INDEX: src->index = g_value_get_int (value); + if (GST_STATE (src) == GST_STATE_NULL) { You'd would need the object lock here. @@ +305,3 @@ + if (GST_STATE (src) == GST_STATE_NULL) { + if (src->index > src->start_index) + src->start_index = src->index; All this is very ambiguous. In Vincent patch, we'd have a initial see to index, while the stream period is still from start to stop. So if you see to zero, you seek to start. In your patch we have a different behaviour, seeking to zero would seek to what index was initially. In fact, start_index is lost. I do think what Vincent patch does is more correct, Has it honor all properties.
Review of attachment 301534 [details] [review]: ::: gst/multifile/gstmultifilesrc.c @@ +302,3 @@ + * several times, as relevant parameters are set (index, caps...) */ + gboolean started = + GST_PAD_MODE (GST_BASE_SRC (src)->srcpad) == GST_PAD_MODE_PUSH; You should read this with the object lock held. @@ +308,3 @@ + + pos = + gst_util_uint64_scale (src->index, src->fps_d * GST_SECOND, src->fps_n); This is not media agnostic. What about picking FORMAT_DEFAULT, and seeking in files ? Also, you didn't protect for the case framerate=0/1.
Anyone of you interesting in updating there patches ?
As per the discussion on IRC, the seek implementation in multifilesrc is broken (seek never get called in multifilesrc because seek event in GST_TIME_FORMAT get rejected by the basesrc class). Seek handling in multifilesrc will be removed later. Once its removed this issue will be resolved.
From discussions here: maybe we should change setting "index" to actually set the start index. That's giving the same behaviour and is much easier to implement.
Created attachment 309220 [details] [review] multifilesrc: fix regression with starting from index set via index property My proposal: setting start_index works as before, and setting index is not supposed to work at runtime anyway, so we might just as well set start_index directly instead. Seems unlikely that anyone will set both "start-index" and "index" properties, even more so to different values.
Pushed slightly more conservative version that checks the STARTED flag (with OBJECT_LOCK, natch): commit 29afa758589620f4572ca2f1bbbac073318df071 Author: Tim-Philipp Müller <tim@centricular.com> Date: Thu Aug 13 17:29:58 2015 +0100 multifilesrc: fix regression with starting from index set via index property When we haven't started yet, set the start_index when we set the index property, so that we start at the right index position after the initial seek. The index property was never really meant to be for writing, but it used to work, so let's support it for backwards compatibility. https://bugzilla.gnome.org/show_bug.cgi?id=739472 (Didn't bother making any of the properties thread-safe, they're not marked as being mutable in playing state anyway.)