GNOME Bugzilla – Bug 514050
Adding subtitles causes video playback to restart
Last modified: 2012-04-23 17:29:59 UTC
It's a nice feature that you can now add subtitles from a file, using the gui. But when you have selected a subtitles file, the playback of the video restarts from the beginning. Is this really necessary? It works that way in mplayer, but isn't Gstreamer capable of switching without such a break?
This is because the xine-lib backend doesn't support it (lowest common denominator). And we won't be able to do it another way until we get rid of xine-lib, or it grows the ability: http://bugs.xine-project.org/show_bug.cgi?id=37
I think gstreamer is more powerful in every aspect. Anyway, here's an idea: If all of subtitles handling were moved to a plugin, there could be two versions of it: subtitles-gstreamer and subtitles-xine. Each with an optimal set of features, including the necessary gui. There are other features of gstreamer that would be nice to deploy also, like being able to position the subtitles anywhere on the canvas. And you can set padding, alignment, wrapping, background box etc. Plus, the main codebase of totem would become somewhat smaller.
Will this be changed in trunk, now that Totem uses Gstreamer exclusively?
It's broken in GStreamer.
Created attachment 139087 [details] [review] instant-sub.patch Patch to use alongside: http://git.gnome.org/cgit/totem/commit/?id=9eab91849905a403ac9309351edba39a3fcb0995 This should allow loading subtitle file on the fly.
+1 for this request.
Supporting this in GStreamer won't be trivial, see bug #589515. For now totem could simply do the following: GstFormat fmt = GST_FORMAT_TIME; GstClockTime position; gst_element_query_position (bvw->priv->play, &fmt, &position); g_object_set (bvw->priv->play, "uri", video_file_uri, "suburi", new_subtitle_uri, NULL); gst_element_set_state (bvw->priv->play, GST_STATE_READY); gst_element_set_state (bvw->priv->play, GST_STATE_PLAYING); (wait until PAUSED) gst_element_seek (bvw->priv->play, ... position ...); Most of the "save position and restore" feature could be reused here. This won't give perfect results but it wouldn't be much work and improve the situation ;) Bastien, what do you think?
Any chance to be able to do this in PAUSED, so as to avoid glitches? Otherwise I'll implement it that way, yes.
(In reply to comment #8) > Any chance to be able to do this in PAUSED, so as to avoid glitches? Otherwise > I'll implement it that way, yes. Well, no :( It can only safely be done in READY. Would it be fine for you if it was possible to be able to set the suburi a single time during playback for a single track? I.e. multiple times would be possible before going to PAUSED and if no suburi was set until then it's possible to set it a single time without doing state changes again. Trying to change an existing suburi during playback would then either be a no-op until the next URI is set or would set the suburi for the next track. I guess I prefer the former. Implementing changing the suburi during playback will result in many problems, see bug #589515.
*** Bug 536087 has been marked as a duplicate of this bug. ***
This is possible on Banshee 1.9.x since they got subtitle supported added. We can select subtitles on fly. Î've been using it without any problems maybe after all this time this should be done in totem.
Ideally this should be done in playbin2 but it's not exactly easy ;) Do you know how this was implemented in banshee?
Banshee did with playbin !? http://git.gnome.org/browse/banshee/commit/?id=9e86951d4234d3abdd25d301837f86475795687e see the initial commit. Afaik banshee moved to playbin2 a long time ago but maybe only for audio files.
Created attachment 212423 [details] [review] Split out the subtitle loading Split out the subtitle loading from _open() so it could eventually be handled on-the-fly (without reloading the movie itself).
Review of attachment 212423 [details] [review]: ::: src/backend/bacon-video-widget-gst-0.10.c @@ +3448,3 @@ * @error: a #GError, or %NULL * * Opens the given @mrl in @bvw for playing. If @subtitle_uri is not %NULL, the given subtitle_uri no longer exists. Should probably still mention that passing in subtitles after #subtitle: in the MRL works (assuming it still does). @@ +3899,3 @@ } +/** bacon_video_widget_set_text_subtitle: Need a new line after the ‘/**’. @@ +3901,3 @@ +/** bacon_video_widget_set_text_subtitle: + * @bvw: a #BaconVideoWidget + * @subtitle_uri: the URI of a subtitle file, or %NULL Needs (allow-none). @@ +3904,3 @@ + * + * Sets the URI for the text subtitle file to be displayed alongside + * the current video. Use %NULL is you * want to unload the current text subtitle “Use %NULL if you*” needs fixing. @@ +3911,3 @@ + const gchar * subtitle_uri) +{ + g_return_if_fail (bvw != NULL); BACON_IS_VIDEO_WIDGET() already does the !=NULL check, so this first assertion is unnecessary.
Created attachment 212499 [details] [review] Split out the subtitle loading and instant apply Split out the subtitle loading from _open() so it can be handled on-the-fly (without reloading the movie itself).
Comment on attachment 139087 [details] [review] instant-sub.patch Merged into the other patch
Comment on attachment 212423 [details] [review] Split out the subtitle loading Fixed the review comments in the new patch.
Review of attachment 212499 [details] [review]: Looks good to me apart from this: ::: src/backend/bacon-video-widget-gst-0.10.c @@ +3908,3 @@ + */ +void +bacon_video_widget_set_text_subtitle (BaconVideoWidget * bvw, I forgot in the other review (sorry), but this should probably be added to totem-sections.txt in the documentation.
See also https://bugzilla.gnome.org/show_bug.cgi?id=661666 for another example of on-the-fly subtitle loading.
Created attachment 212568 [details] [review] Split out the subtitle loading and instant apply Split out the subtitle loading from _open() so it can be handled on-the-fly (without reloading the movie itself).
A couple of different problems: - We don't seem to be getting an updated list of channels, so we don't see the subtitles being listed - I'm not certain the TEXT flag is getting set either We also probably need to check the index-ing of subtitles when both internal and external subtitles are available.
Created attachment 212598 [details] [review] backend: Split out the subtitle loading Split out the subtitle loading from _open() so it can be handled on-the-fly (without reloading the movie itself).
Hacked around the playbin2 limitations, similarly to how Banshee did it. Causes a slight glitch in display, but it's better than restarting all the way from the beginning. Attachment 212598 [details] pushed as 65c83b0 - backend: Split out the subtitle loading
Thanks Bastien, i owe you a beer!