After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 499463 - Allow setting text subtitles from the view menu
Allow setting text subtitles from the view menu
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: Movie player
unspecified
Other Linux
: Normal normal
: ---
Assigned To: General Totem maintainer(s)
General Totem maintainer(s)
: 533853 552236 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-11-25 01:13 UTC by Bastien Nocera
Modified: 2008-12-10 19:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Adding selection subtitle in view menu (4.53 KB, patch)
2007-11-26 15:56 UTC, Kamil Pawlowski
needs-work Details | Review
Subtitle in menus (6.35 KB, patch)
2008-06-15 20:53 UTC, Kamil Pawlowski
needs-work Details | Review
Select subtitles in view menu (5.92 KB, patch)
2008-12-10 15:50 UTC, Kamil Pawlowski
needs-work Details | Review
totem-select-text-subs.patch (8.71 KB, patch)
2008-12-10 19:10 UTC, Bastien Nocera
none Details | Review

Description Bastien Nocera 2007-11-25 01:13:56 UTC
It would make sense for videos without builtin subtitles.
Comment 1 Kamil Pawlowski 2007-11-26 15:56:50 UTC
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)?
Comment 2 Michael Monreal 2008-01-09 09:20:37 UTC
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).
Comment 3 Bastien Nocera 2008-01-10 10:28:52 UTC
(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.
Comment 4 Bastien Nocera 2008-05-19 12:25:05 UTC
*** Bug 533853 has been marked as a duplicate of this bug. ***
Comment 5 Kamil Pawlowski 2008-06-15 20:53:02 UTC
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.
Comment 6 Bastien Nocera 2008-12-05 15:30:00 UTC
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.
Comment 7 Bastien Nocera 2008-12-05 16:24:32 UTC
*** Bug 552236 has been marked as a duplicate of this bug. ***
Comment 8 Kamil Pawlowski 2008-12-10 12:23:30 UTC
> 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.
Comment 9 Kamil Pawlowski 2008-12-10 12:24:55 UTC
> 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.
Comment 10 Bastien Nocera 2008-12-10 13:01:27 UTC
(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.
Comment 11 Kamil Pawlowski 2008-12-10 15:50:20 UTC
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.
Comment 12 Bastien Nocera 2008-12-10 18:42:01 UTC
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.
Comment 13 Bastien Nocera 2008-12-10 19:10:27 UTC
Created attachment 124376 [details] [review]
totem-select-text-subs.patch

Will commit that after more thorough checks and cleanups.
Comment 14 Bastien Nocera 2008-12-10 19:46:10 UTC
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)