GNOME Bugzilla – Bug 499463
Allow setting text subtitles from the view menu
Last modified: 2008-12-10 19:46:10 UTC
It would make sense for videos without builtin subtitles.
Created attachment 99669 [details] [review] Adding selection subtitle in view menu That works. But in playlist.c there is now 2 very similar functions: playlist_select_subtitle_action_callback and totem_playlist_select_subtitle. Maybe I should make third function and in that 2 makes only call to that third (with different iter)?
This would be great to have, I was just looking for this... opening the playlist just for setting subtitles is not very nice and also not discoverable at all (I just knew about it because I read the SVN log).
(In reply to comment #1) > Created an attachment (id=99669) [edit] > Adding selection subtitle in view menu <snip> > Maybe I should make third function and in that 2 makes only call to that third > (with different iter)? Sounds like a good plan.
*** Bug 533853 has been marked as a duplicate of this bug. ***
Created attachment 112799 [details] [review] Subtitle in menus Check this, this patch provide subtitle selection item in menu view and video context menu. New function totem_playlist_subtitle_select_dialog can be used to select subtitle for current playing or current selected on playlist item.
It looks alright except that: - it should only show the menu entry when no subtitles are builtin to the file - it shouldn't show the menu entry when playing DVDs or VCDs - You're leaking the GList in totem_playlist_select_subtitle_dialog() and the GList isn't used outside the "if (mode == TOTEM_PLAYLIST_DIALOG_SELECTED)" conditional, so define it there. - There's extra line feeds around the patch: + gtk_tree_model_get_iter (playlist->priv->model, &iter, playlist->priv->current); + + } else if (mode == TOTEM_PLAYLIST_DIALOG_SELECTED) { Hopefully next time I won't take so long to review the patch.
*** Bug 552236 has been marked as a duplicate of this bug. ***
> it should only show the menu entry when no subtitles are builtin to the file This is not good if program try to be that smart. Many times builtin subtitles are not in needed language.
(In reply to comment #9) > > it should only show the menu entry when no subtitles are builtin to the file > > This is not good if program try to be that smart. Many times builtin subtitles > are not in needed language. Fair enough, the rest still stands though.
Created attachment 124355 [details] [review] Select subtitles in view menu This is new patch, I am not sure of the method detecting the type of stream which I use (parsing mrl). If You could, fix my english in comments.
You should really be using SVN instead of tarballs, that would save you from forgetting the src/totem-playlist.h file, and you don't even need to install totem to use it from the src directory.
Created attachment 124376 [details] [review] totem-select-text-subs.patch Will commit that after more thorough checks and cleanups.
2008-12-10 Bastien Nocera <hadess@hadess.net> * data/totem.ui: * src/totem-menu.c (select_subtitle_action_callback): * src/totem-playlist.c (totem_playlist_select_subtitle_dialog), (playlist_select_subtitle_action_callback): * src/totem-playlist.h: * src/totem-uri.c: * src/totem-uri.h: * src/totem.c (totem_action_set_mrl_with_warning): Patch from Kamil Pawlowski <kamilpe@gmail.com> to allow selecting a text subtitle for the currently playing movie, from the View menu (Closes: #499463)