GNOME Bugzilla – Bug 618746
Patch to add accurate seeking in totem_action_seek_time () and totem_action_seek_relative () functions
Last modified: 2012-04-23 17:10:59 UTC
Add third parameter 'gboolean accurate' to bacon_video_widget_seek_time () in src/backend/bacon-video-widget-gst-0.10.c to use GST_SEEK_FLAG_ACCURATE instead of GST_SEEK_FLAG_KEY_UNIT for accurate seeking (maybe slower on some formats) if set to TRUE Add forth parameter 'gboolean accurate' to totem_seek_time_rel () in src/totem-object.c for accurate seeking if set to TRUE, as it used by other functions: totem_action_seek_time () and totem_action_seek_relative () Add third parameter 'gboolean accurate' to totem_action_seek_relative () in src/totem-object.c for accurate seeking if set to TRUE, as it can be used from plugins Add third parameter 'gboolean accurate' to totem_action_seek_time () in src/totem-object.c for accurate seeking if set to TRUE, as it can be used from plugins Rename second parameter 'gint64 sec' to 'gint64 msec' in totem_action_seek_time () in src/totem-object.c for the more clear meaning Update all functions which uses mentioned above functions Set 'gboolean accurate' parameter to FALSE everywhere except the 'skipto' plugin (it should be more precisely) Update Python and Vala bindings in order of new 'gboolean accurate' parameter
Created attachment 161137 [details] [review] Add third parameter 'gboolean accurate' to bacon_video_widget_seek_time () in src/backend/bacon-video-widget-gst-0.10.c to use GST_SEEK_FLAG_ACCURATE instead of GST_SEEK_FLAG_KEY_UNIT for accurate seeking (maybe slower on some formats) if set to TRUE Add
The actual problem was that if you are using totem_action_seek_time () function from the plugin, the positioning is not much accurate 'cause of using GST_SEEK_FLAG_KEY_UNIT. Patch adds 'gboolean accurate' parameter to this function (and totem_action_seek_relative (), too) which forces to use GST_SEEK_FLAG_ACCURATE if set to TRUE.
Review of attachment 161137 [details] [review]: The commit message is supposed to use a short summary (under 72 characters long), and then a long description of the changes. ::: src/backend/bacon-video-widget-gst-0.10.c @@ +3887,3 @@ * @bvw: a #BaconVideoWidget * @_time: the time to which to seek, in milliseconds + * @accurate: weather to use accurate seek whether. Add "More accurate seeks are slower". Or something akin to that, see the GStreamer definition for the flag. @@ +3920,3 @@ gst_element_seek (bvw->priv->play, FORWARD_RATE, + GST_FORMAT_TIME, + GST_SEEK_FLAG_FLUSH | (accurate ? GST_SEEK_FLAG_ACCURATE : GST_SEEK_FLAG_KEY_UNIT), could you split that into a separate line. flag = ...; GST_SEEK_FLAG_FLUSH | flag; ::: src/totem-object.c @@ +1958,3 @@ * @totem: a #TotemObject * @offset: the time offset to seek to + * @accurate: weather to use accurate seek (maybe slower) Whether. "May be" is 2 words. @@ +1973,3 @@ * @totem: a #TotemObject + * @msec: the time to seek to + * @accurate: weather to use accurate seek (maybe slower) Whether. Same comment as above. ::: src/totem-video-thumbnailer.c @@ +430,3 @@ GError *err = NULL; + if (bacon_video_widget_seek_time (bvw, seconds * 1000, FALSE, &err) == FALSE) { Should be TRUE.
Created attachment 161280 [details] [review] Fixed code mentioned in review Hope I fix all bad things from previous patch.
(In reply to comment #4) > Created an attachment (id=161280) [details] [review] > Fixed code mentioned in review > > Hope I fix all bad things from previous patch. *fixed
Created attachment 161326 [details] [review] Add support for accurate seek Add "accurate" argument to seek functions, and use GST_SEEK_FLAG_ACCURATE instead of GST_SEEK_FLAG_KEY_UNIT when an accurate seek is requested. All the users of the seek functions will request "inaccurate" seeks except the video thumbnailer and the skipto plugin.
I fixed up the commit message, as seen in the attachment. Rest looked good, thanks! Attachment 161326 [details] pushed as e1332de - Add support for accurate seek
Just for the record, accurate seeks are not just "maybe slower on some formats". Accurate seeking means: do whatever it takes to make this accurate, even if it involves scanning/decoding the entire file from the start; even if it takes hours; etc.. This is currently not really implemented properly yet, but in future you may find things getting considerably slower if you pass the accurate flag. This is generally not desirable for the thumbnailer or the skipto function. There's a third option btw, which is to pass neither the ACCURATE flag nor the KEY_UNIT flag to the seek. In that case the seek should basically behave similar to a key unit seek: data will usually be pushed from the previous key frame, but unlike in the case of the KEY_UNIT seek, the new start time will not be adjusted to the position of the key frame, but kept as the seek time specified. This usually means the content between the key buffer and the seek position will be clipped (decoded, but not actually output). The difference to ACCURATE seeking is, for example, that an mpeg demuxer could still estimate the new position based on the average bitrate in that case, but with the ACCURATE flag it would be expected to scan the file from the beginning (unless it has built an internal index already). Note that not all demuxers/parsers honour these three seek options 100% correctly yet. Please file bugs if you notice elements that don't.
I pushed this: commit f0796f0d4f1a1cf2b12c9bc5149ec96e6d84d87d Author: Bastien Nocera <hadess@hadess.net> Date: Mon Apr 23 18:09:04 2012 +0100 backend: Don't use keyframe seeks for inaccurate seeks Because we actually want those seeks to be rather quick, but we don't need them to be frame perfect, so seek without any flags rather than with GST_SEEK_FLAG_KEY_UNIT. See https://bugzilla.gnome.org/show_bug.cgi?id=618746#c8 for a longer explanation