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 165981 - Graphic selection of the Subtitles
Graphic selection of the Subtitles
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: General Totem maintainer(s)
General Totem maintainer(s)
: 169952 329327 337871 354732 394853 513732 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2005-02-01 22:14 UTC by Bartosz
Modified: 2008-02-01 17:47 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
subtitle_mockup (23.81 KB, image/png)
2005-08-16 17:33 UTC, Teppo Turtiainen
  Details
subtitles patch (9.92 KB, patch)
2006-03-19 10:22 UTC, Kamil Pawlowski
needs-work Details | Review
strcpy to strdup and using subtitles_ext (9.91 KB, patch)
2006-03-19 16:27 UTC, Kamil Pawlowski
needs-work Details | Review
totem graphic selection of the subtitles (18.61 KB, patch)
2007-11-22 00:26 UTC, Kamil Pawlowski
needs-work Details | Review
totem graphic selection of the subtitles (20.20 KB, patch)
2007-11-23 11:13 UTC, Kamil Pawlowski
none Details | Review

Description Bartosz 2005-02-01 22:14:39 UTC
Hi.
I think Totem is the one of the best player for Linux.

But he haven't got graphic selection of the subtitles.
Most of CD films is with subtitles. 
There is no standard video player with subtitles. Why ?
Is this so hard to make it ?
Comment 1 Ronald Bultje 2005-02-01 22:35:50 UTC
What do you mean? Subtitles in most movie formats (Ogm, Matroska) are
automatically selected and you can switch between subtitles using the menubar.
For AVI, if the external subtitle file has the same name as the .avi file
(s/avi/sub/), the same happens. Or do you want to be able to graphically select
subtitle files after selecting an AVI file?
Comment 2 Bartosz 2005-02-01 22:45:07 UTC
I would like playing AVI movies with subtitles.
Most of times, the subtitles is in another directory or they have another name 
(that movie file).
I would like to select the subtitle file, from the Totem.
It is difficult to change name/location every You play a new AVI movie.

If this option will be added, the Totem will be first standard graphical movie 
Player .

For me it is very important, because in Poland most of AVI movies is with 
subtitles. 
Comment 3 Bartosz 2005-02-01 22:49:29 UTC
Most of my friends, didn't even know, that Totem support subtitles.
(Thats why they use MPlayer)
On discussion group, when somebody ask how display Totem subtitles, 
they told to download MPlayer.
Comment 4 Bartosz 2005-02-01 22:51:14 UTC
It is possible to change size of the font ?
Comment 5 Ronald Bultje 2005-02-01 23:21:03 UTC
Depends on backend. Xine backend: no clue. GStreamer backend: not yet. It's
easily added, that's why we made it a "task", which is intended for new users to
become familiar with GStreamer programming (see
http://gstreamer.freedesktop.org/tasks/gnome.html#fontselection). It'd take me
1-2 hours, but that's not the point. I want other people to become familiar with
it. Some people have shown interest, but I haven't seen a code submission yet.

If you want subtitles, do open location,
"file:///path/to/file.avi#subtitle:/path/to/subtitle.srt". It's not exactly
graphical, but quick enough (note: this is from memory, it may be slightly
different).

If you have suggestions on how we can add this to the UI in a clueful way, I'll
code something up, but right now I can't think of a quick'n'simple way to select
subtitles when opening a movie.
Comment 6 Bastien Nocera 2005-02-01 23:47:42 UTC
You can change the side of the font with the xine-lib backend, check in
~/.gnome2/totem_config
In all cases, if your subtitles are in another directory from the original film,
Totem won't know about it. The xine-lib backend can open subtitles by using the
command line referred to on the Totem home page.
If you have any other ideas on how to introduce this concept in the UI cleanly,
please tell us, otherwise, you can probably write 10 lines of Perl/Python GTK+
and have a UI able to feed Movies+subtitles to Totem with any problems.
Comment 7 Bartosz 2005-02-01 23:57:25 UTC
I would like in "Movie" menu, one new choice "open subtitle".  
It can be like in "Open _Location...". Then You can enter path to subtitle. 
 
And (if it possible) in "preferences" new option "font size" 
Comment 8 Ronald Bultje 2005-02-02 00:42:44 UTC
Bastien, maybe a weird idea (not totally what it's intended for), but we could
enable multiple file selection in the dialog. Then, at least if they're in the
same dir, you select both files and get stuff automagically.
Comment 9 Bartosz 2005-02-02 01:39:53 UTC
Great !!! 
That I want !!! 
Comment 10 Bartosz 2005-02-08 19:05:03 UTC
When will be a new Totem release, with support of subtitles (graphic 
selection) ?
Comment 11 Bastien Nocera 2005-06-30 19:46:17 UTC
*** Bug 169952 has been marked as a duplicate of this bug. ***
Comment 12 Teppo Turtiainen 2005-08-16 17:31:50 UTC
I too would really like to see this feature added to Totem and I've been trying
to come up with a clean way of doing it. Here is a little "minispec" for one
possible way:

1. Allow adding subtitle files to the playlist using all available methods
(dragging from Nautilus, clicking the Add button, choosing Movie > Open).

2. If some modifier key (Ctrl?) is pressed while dragging a subtitle file from
Nautilus or from elsewhere in the playlist, change the cursor to indicate this
(plus sign?) and allow dragging the subtitle file over a movie file and then
associate the subtitle file with that movie file.

3. If a movie file is selected when clicking the Add button or choosing Movie >
Open and selecting a subtitle file, associate the subtitle file with that movie
file.

4. Clearly indicate that the subtitle file is a subordinate of the movie file
(see attached mockup for one possible way).

5. If a subtitle file is added to the playlist but not associated with any movie
file, silently ignore it.

What do you guys think?
Comment 13 Teppo Turtiainen 2005-08-16 17:33:34 UTC
Created attachment 50804 [details]
subtitle_mockup
Comment 14 Andrey Tatarinov 2005-08-16 17:46:19 UTC
As a user I think the most intuitive way is to drag subtitles file to the video
widget or to the movie entry in playlist (without pressing any keys or smth).

Also having subtitles files in playlist while they do not have any meaning there
is not right, I think.
Comment 15 Fabio Bonelli 2006-01-31 14:18:41 UTC
*** Bug 329327 has been marked as a duplicate of this bug. ***
Comment 16 Matthias Saou 2006-01-31 15:00:18 UTC
Mostly a "me too" here, just to say that playing video files which have external subtitles with Totem is simply non functional for a lot of people (including me), since you can't expect people to run Totem from the command line, nor always have the subtitle file's basename match the video's : I for instance often have subtitles in more than one language for one given video, and they obviously can't be called the same, nor do I want to have the rename one or more each time I want a particular subtitle loaded.

Totem clearly lacks at least a menu entry for loading a subtitle file for the currently opened video (and a way to then drop it, and select/force the encoding). Ideally, I'd like to simply be able to drag a subtitle file from nautilus onto the playing video and instantly have it loaded and enabled. Adding some kind of second playlist for the subtitles could also be an option, or as Teppo suggested, be able to "associate" subtitle files with video directly inside the playlist (seems quite neat and simple, actually... just what I'm looking for).

Matthias
Comment 17 Lionel Dricot 2006-02-20 14:46:22 UTC
Following http://dolphy-tech.net/log/?p=26 , it seems that this bug is now fixed. Am I wrong ?
Comment 18 Matthias Saou 2006-02-20 15:03:48 UTC
Unfortunately you are indeed wrong :-(
He wrote "with subtitles from a muxed format", so if you have .srt, .ssa or similar subtitles muxed with their video in a matroska container for instance, it'll work since the subtitles will be detected and their selection enabled.

But for subtitles in external files, there is still no GUI path to get them loaded and enabled... and AFAIK, the separate .srt file is still the most common case.
Comment 19 Lionel Dricot 2006-02-20 15:05:54 UTC
"Seeking now works perfectly in Totem with subtitles from a file AND with subtitles from a muxed format."

Reading "from a file", I tought that srt, sub, etc was supported.
Comment 20 Matthias Saou 2006-02-20 15:14:16 UTC
Ah, sorry, I also read it wrong! Anyway, it's seeking in the video that now works with subtitles, but the subtitle selection will only be available for subtitles muxed inside the file that has been opened, there is still no "Open subtitle file..." menu entry or similar.

I would really love to see a "Load subtitle from file..." item in that subtitle selection menu, though. Anyone? Please!?
Comment 21 Julien MOUTTE 2006-02-20 15:36:20 UTC
Yeah you misundertood me ! :)

There still needs to be a UI change to select subtitles from file in an easier way.

Bastien any plans on that ?
Comment 22 Bastien Nocera 2006-02-20 18:34:22 UTC
From me, certainly not. If anyone can come up with a decent UI, feel free to present it. Also bear in mind that xine-lib only supports opening subtitles when opening the file on startup, not during playback.
Comment 23 Matthias Saou 2006-02-20 18:39:16 UTC
Then for the xine-lib backend, couldn't the behavior just be like in MPlayer, where the playback stops when a subtitle is loaded? Better than nothing...
Comment 24 Bastien Nocera 2006-03-08 16:41:29 UTC
*** Bug 333924 has been marked as a duplicate of this bug. ***
Comment 25 Lukáš 'Spike' Polívka 2006-03-12 14:06:12 UTC
Totem certainly lacks UI option to load external subtitles AND what's more -- ability to load subtitles with filenames like "movie_name.lang.srt", while movie filename is "movie_name.ext". The "lang" part should then be used in UI, in View>Subtitles, to allow user to select from multiple languages if more subtitle files are present.

These features are kind of common and useful in Windows enviroment -- for example in Media Player Classic.
Comment 26 Kamil Pawlowski 2006-03-19 10:22:28 UTC
Created attachment 61533 [details] [review]
subtitles patch

This patch provide graphic selection of the subtitles to Totem-1.4.0. Subtitles can be part of mrl like: movie_name.avi#subtitle:subtitles.txt. This feature can help users who open movie from CD disc when change of files name can't be possible
Comment 27 Lukáš 'Spike' Polívka 2006-03-19 12:46:30 UTC
I have tried the patch. It hangs my Totem 1.4.0 (Gentoo).
It also seems to try reloading the whole graph/sink (because the text renderer wasn't present...), which kind of ruins the whole experience.
Comment 28 Bastien Nocera 2006-03-19 13:06:56 UTC
Comment on attachment 61533 [details] [review]
subtitles patch

>Index: src/totem-uri.c
>===================================================================
>--- src/totem-uri.c	(.../trunk)	(wersja 10)
>+++ src/totem-uri.c	(.../branches/totem-subtitles)	(wersja 10)
<snip>
>+	len += strlen(full_movie)+1;
>+	mrl = g_malloc(len);
>+	strcpy(mrl,full_movie);

Don't use strcpy, use g_strdup_printf instead.

>+	

Mind the spurious tabs.

<snip>
>+	filter_subtitles = gtk_file_filter_new ();
>+	gtk_file_filter_set_name (filter_subtitles, _("Subtitles"));
>+	gtk_file_filter_add_pattern (filter_subtitles, "*.asc");
>+	gtk_file_filter_add_pattern (filter_subtitles, "*.txt");
>+	gtk_file_filter_add_pattern (filter_subtitles, "*.sub");
>+	gtk_file_filter_add_pattern (filter_subtitles, "*.srt");
>+	gtk_file_filter_add_pattern (filter_subtitles, "*.smi");
>+	gtk_file_filter_add_pattern (filter_subtitles, "*.ssa");
>+	gtk_file_filter_add_pattern (filter_subtitles, "*.txt");
>+	g_object_ref (filter_subtitles);

Why not use the contents of the subtitle_ext array instead?

As for the rest of the patch, I'd rather not wedge the subtitle on the end of the URI for the movie. I would prefer if the subtitle was added to another (hidden) column in the playlist's List store.
Also, the GStreamer backend can load those files on the fly, and from remote locations, whereas xine-lib can't (although it should be possible to change this). What happens when you're playing a movie and changing its subtitle?
Comment 29 Lukáš 'Spike' Polívka 2006-03-19 13:20:02 UTC
I forgot to write: I have GStreamer 0.10.4 backend. It hangs when I add the subtitles using the Add Subtitles option.
Adding subtitles should really be possible on-the-fly.
Comment 30 Kamil Pawlowski 2006-03-19 16:27:57 UTC
Created attachment 61549 [details] [review]
strcpy to strdup and using subtitles_ext

full mrl was saved in playlist files, and be parsed from command line. But i can change this to hidden column, but what gonna do to playlist and command line ?

When i testing gstreamer backend (with totem and totem without patch) i have crash when - change playlist item. And "Add subtitles" function sends the same signals like treeview_row_changed function. I am not sure, maybe this is gstreamer problem? 

But xine works fine on my installation. Xine cant load subtitles on the fly like gstreamer. So "Add subtitles" on gstreamer must sends different signal?
Comment 31 Bastien Nocera 2006-05-10 14:39:56 UTC
*** Bug 337871 has been marked as a duplicate of this bug. ***
Comment 32 Bastien Nocera 2006-05-28 11:03:40 UTC
Comment on attachment 61549 [details] [review]
strcpy to strdup and using subtitles_ext

>Index: src/totem-uri.c
>===================================================================
>--- src/totem-uri.c	(.../trunk)	(wersja 12)
>+++ src/totem-uri.c	(.../branches/totem-subtitles)	(wersja 12)
>@@ -36,6 +36,7 @@
> 
> static GtkFileFilter *filter_all;
> static GtkFileFilter *filter_supported;
>+static GtkFileFilter *filter_subtitles;
> 
> gboolean
> totem_playing_dvd (const char *uri)
>@@ -87,8 +88,34 @@
> 	return (S_ISBLK (buf.st_mode));
> }
> 
>+char * totem_create_full_path (const char *path)

There's already a totem_create_full_path(). What's the difference with this one? Could you find a better name for it?


> char*
>-totem_create_full_path (const char *path)
>+totem_create_one_full_path (const char *path)

Same here.

> char *
>+totem_uri_get_movie_uri (const char *uri)
>+{
>+	char *movie = NULL, *after_q = NULL;
>+	int len;
>+
>+	if ((after_q = strstr (uri, "#subtitle:")) != NULL) {
>+		len = after_q - uri;
>+		movie = g_malloc(len);
>+		strncpy(movie,uri,len);
>+		movie[len] = '\0';

Use g_strndup here instead.

>+gchar *
>+totem_add_subtitle (GtkWindow *parent, const char *path, char **new_path)
>+{
>+	gchar *filename;
>+	GtkWidget *fs;
>+	int response;
>+	
>+	fs = gtk_file_chooser_dialog_new (_("Select subtitles file"),
>+	   parent,
>+	   GTK_FILE_CHOOSER_ACTION_OPEN,
>+	   GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
>+	   GTK_STOCK_ADD, GTK_RESPONSE_ACCEPT,
>+	   NULL);
>+
>+	gtk_dialog_set_default_response(GTK_DIALOG (fs), GTK_RESPONSE_ACCEPT);
>+	gtk_file_chooser_set_local_only (GTK_FILE_CHOOSER (fs), FALSE);
>+	gtk_file_chooser_add_filter (GTK_FILE_CHOOSER (fs), filter_all);
>+	gtk_file_chooser_add_filter (GTK_FILE_CHOOSER (fs), filter_subtitles);
>+	gtk_file_chooser_set_filter (GTK_FILE_CHOOSER (fs), filter_subtitles);
>+
>+	if (path != NULL) {
>+		gtk_file_chooser_set_current_folder_uri(GTK_FILE_CHOOSER(fs), 
>+						path);
>+	}
>+	
>+	response = gtk_dialog_run(GTK_DIALOG(fs));
>+	gtk_widget_hide (fs);
>+	while (gtk_events_pending())
>+		gtk_main_iteration();

No need for that, the functions afterwards are fast enough to make sure the dialogue disappears quickly.

> char*		totem_create_full_path	(const char *path);
>+char*		totem_create_one_full_path (const char *path);

Same question as at the top.

> static void
>+on_subtitles_add_activate (GtkButton *button, TotemPlaylist *playlist)
>+{
>+	GList *l;
>+	GtkTreePath *path;
>+	GtkTreeIter iter;
>+	char *mrl, *movie, *url;
>+	char *new_path;
>+	gchar *subtitle;
>+
>+	subtitle = totem_add_subtitle (totem_playlist_get_toplevel (playlist),
>+					playlist->_priv->path, &new_path);
>+	if (subtitle == NULL) return;

2 lines please.

>+	
>+	if (new_path != NULL) {
>+		g_free (playlist->_priv->path);
>+		playlist->_priv->path = new_path;
>+	}
>+
>+	l = gtk_tree_selection_get_selected_rows(playlist->_priv->selection,
>+		NULL);
>+	path = l->data;
>+
>+	gtk_tree_model_get_iter(playlist->_priv->model, &iter, path);
>+	gtk_tree_model_get(playlist->_priv->model, 
>+		&iter, 
>+		URI_COL, 
>+		&url, 
>+		-1);
>+
>+	movie = totem_uri_get_movie_uri(url);
>+	mrl = g_strdup_printf("%s#sbutitle:%s",movie,subtitle);
>+	

Mind the spurious tabs.

>+	gtk_list_store_set (GTK_LIST_STORE(playlist->_priv->model), 
>+		&iter, URI_COL, mrl, -1);
>+	

And again.

>+	if (totem_playlist_gtk_tree_path_equals(
>+		path, playlist->_priv->current) != FALSE)

Don't break function calls after the bracket, and make sure there's a space before the bracket.

>+static void
> on_copy1_activate (GtkButton *button, TotemPlaylist *playlist)
> {
> 	GList *l;
> 	GtkTreePath *path;
> 	GtkClipboard *clip;
>+	GtkTreeIter iter;
> 	char *url;
>-	GtkTreeIter iter;

Completely spurious change.
Comment 33 Tim-Philipp Müller 2006-06-13 10:24:36 UTC
> I forgot to write: I have GStreamer 0.10.4 backend. It hangs when I add the
> subtitles using the Add Subtitles option.

Do you still get this with current GStreamer core/base (this might have been bug #343397)?



Comment 34 Sergej Kotliar 2006-09-07 08:05:08 UTC
*** Bug 354732 has been marked as a duplicate of this bug. ***
Comment 35 tomi 2006-09-07 08:30:46 UTC
FOr all those saying that XINE doesnt support adding subs on the fly well it does.
So far I've tested it with XINE-UI and it works great. Even better than in MPLAYER. Since it doesn't restart the movie when U add a subtitle but it just continuous on.

Any ideas when this feature will be implemented and is there a hack I could use that would allow me to add SUBS on the fly?
Comment 36 tomi 2006-09-07 11:20:55 UTC
If/when this feature will be implemented it should also allow drag and drop.So U could just drag and drop them from the ZIP file that U usually download them from.
Comment 37 Bastien Nocera 2006-09-07 11:32:46 UTC
(In reply to comment #35)
> FOr all those saying that XINE doesnt support adding subs on the fly well it
> does.

No, it doesn't. It reopens the file with the subtitle parameter. GStreamer can use those subtitles without restarting the playback.
Comment 38 Matthias Saou 2006-09-07 11:41:02 UTC
(In reply to comment #37)
> GStreamer can use those subtitles without restarting the playback.

Then... "Just do it" :-)

If it means starting with drag'n drop for on-the-fly subtitles, and only later adding some dialog to do the same, that would be fine. Any solution that doesn't require running Totem from the CLI would be a great improvement.

Matthias
Comment 39 Tim-Philipp Müller 2006-09-07 11:54:38 UTC
My feeling is that all this 'graphical selection' stuff is a bit over the top and not particularly discoverable either. As a start, I think something like adding a

  [x] read subtitles from external file

checkbox to the open dialog would already improve the situation quite a bit (totem could then pop up a second dialog asking for the subtitle file, for example; it should also be possible to get drag-and-drop-from-archive-into-filechooser working somehow I think).
Comment 40 tomi 2006-09-07 12:57:14 UTC
TIM-PHILIP
I second that!

BASTIEN
I don't know about GSTREAMER but XINE-UI doesn't restart the playback.

Just tried, I added a new external subtitle in about 30minutes of watching a film and when I added the new subtitle it continued from the 30minutes of watching it!
Comment 41 Bastien Nocera 2006-09-07 13:42:35 UTC
(In reply to comment #40)
> BASTIEN
> I don't know about GSTREAMER but XINE-UI doesn't restart the playback.
> 
> Just tried, I added a new external subtitle in about 30minutes of watching a
> film and when I added the new subtitle it continued from the 30minutes of
> watching it!

Yes, it reopens the film...
Look at the code in xine-ui/src/xitk/actions.c:
In gui_select_sub(), it will save the current position (in a "mediamark"), and in subselector_callback(), it will do:
          xine_close (gGui->spu_stream);

          if(xine_open(gGui->spu_stream, mmk->sub))
            xine_stream_master_slave(gGui->stream,
                                     gGui->spu_stream, XINE_MASTER_SLAVE_PLAY | XINE_MASTER_SLAVE_STOP);

          if(gui_xine_get_pos_length(gGui->stream, &curpos, NULL, NULL)) {
            xine_stop(gGui->stream);
            gui_set_current_position(curpos);
          }

Which is why you see the OSD you usually get when seeking when you open the subtitle file. Imagine doing that with a high-latency stream, and it will take absolute ages to reopen. So, again, it *does* reopen the file.
Comment 42 Matthias Saou 2006-09-07 13:50:35 UTC
(In reply to comment #41)
> Yes, it reopens the film...
> Look at the code in xine-ui/src/xitk/actions.c: [...]

Well, then do the same hack for the xine backend if you need to, and do it the right "on-the-fly" way for the gstreamer backend. Cool. Further questions? :-)

Matthias
Comment 43 Teppo Turtiainen 2006-09-07 13:58:21 UTC
(In reply to comment #41)
> Which is why you see the OSD you usually get when seeking when you open the
> subtitle file. Imagine doing that with a high-latency stream, and it will take
> absolute ages to reopen. So, again, it *does* reopen the file.

Adding a subtitle to a high-latency stream would be inconvenient, but I don't quite understand why it *prevents* adding this feature.
Comment 44 Bastien Nocera 2006-09-07 15:00:36 UTC
*Sigh*
The patch isn't ready, and the UI is borderline acceptable (not easily discoverable). So please stop badgering on about how the feature isn't merged in. This feature is far from being my priority.

Fix the patch, test whether it hangs with the GStreamer backend or not, make sure the UI is better discoverable, then the feature will be added.
Comment 45 tomi 2006-09-07 15:44:03 UTC
BASTIEN
Tanx 4 explaining me about the XINE-UI thing!

So ATM theres no GNOME VIDEO player that would handle opening EXTERNAL SUBTITLES on the fly except VLC which doesn't support putting subtitles bellow the movie.

But I am very puzzled, why at least the following couldn't be in TOTEM:
Too add an external subtitle even if it starts the movie from the beginning. That way there would be no problems with STREMING VIDEO.

And I believe that TOTEM-XINE would then become the first GTK2 player that would support the following:
putting subtitles bellow the movie,changing their colours and height,changing their CP,opening the external subtitle.
Comment 46 Bastien Nocera 2006-10-23 15:46:49 UTC
Add the usability keyword so we can get some feedback on how to implement the UI for this feature properly.
Comment 47 tomi 2006-10-23 16:05:35 UTC
BASTIEN
What do U mean by, and I quote:
"Add the usability keyword so we can get some feedback on how to implement the
UI for this feature properly."
How do I do that, cause I don't understand what U want.
Comment 48 Bastien Nocera 2006-10-23 16:06:53 UTC
(In reply to comment #47)
> BASTIEN
> What do U mean by, and I quote:
> "Add the usability keyword so we can get some feedback on how to implement the
> UI for this feature properly."
> How do I do that, cause I don't understand what U want.

That's what I just did, not what I'm asking for.
Comment 49 Bastien Nocera 2007-01-26 14:58:41 UTC
*** Bug 394853 has been marked as a duplicate of this bug. ***
Comment 50 amit.man 2007-07-09 20:56:49 UTC
Hello.

this feature is very important for me too.
any chance it will be added?

thanks
amit
Comment 51 tomi 2007-07-28 15:03:21 UTC
AMIT
Well I have given up on this. 

I suggest to all of U to use KAFFEINE which has excellent subtitle support and works great in GNOME too.
Comment 52 David Prieto 2007-10-23 11:39:17 UTC
Any chance to get a "view → subtitles → load file" menu entry?
Comment 53 Kamil Pawlowski 2007-11-22 00:26:38 UTC
Created attachment 99459 [details] [review]
totem graphic selection of the subtitles

Hi guys I have some nice patch (again). This patch is very beta but works for me. 

He use hidden column in playlist for subtitle path
I not tested save or load playlist yet

Bastien please check this, I think this feature must be implemented!
Maybe I should do not touch the totem main functions because they are binding and this make incompatibility? But I think this is just the better way to send play/stop commands.

(for version 2.21.0)
Comment 54 Bastien Nocera 2007-11-23 02:32:44 UTC
Comment on attachment 99459 [details] [review]
totem graphic selection of the subtitles

>diff -Naur totem-2.21.0.orig/data/playlist.ui totem-2.21.0/data/playlist.ui

Please do your diffs against SVN, there's plenty of changes in development versions, and you're 2 releases behind in the diff.

<snip>
>+               <property name="label">_Select subtitle</property>

Should be "Select text subtitle..."

>+               <property name="tooltip">Select file with subtitles</property>

Not used ATM, but "Select a file to use for text subtitles" is probably better

<snip>
>+         <menuitem name="load-subtitles" action="select-subtitles"/>

Is it load-subtitles, or select-subtitles?

>diff -Naur totem-2.21.0.orig/src/plugins/mythtv/totem-mythtv.c totem-2.21.0/src/plugins/mythtv/totem-mythtv.c
>--- totem-2.21.0.orig/src/plugins/mythtv/totem-mythtv.c	2007-10-19 13:28:39.000000000 +0200
>+++ totem-2.21.0/src/plugins/mythtv/totem-mythtv.c	2007-11-21 18:19:43.000000000 +0100
>@@ -231,7 +231,7 @@
> 			    URI_COL, &uri, -1);
> 	g_return_if_fail (uri != NULL);
> 
>-	totem_action_set_mrl_and_play (plugin->totem, uri);
>+	totem_action_set_mrl_and_play (plugin->totem, uri, NULL);

That's not there anymore.

> 		mrl = totem_playlist_get_current_mrl (totem->playlist);
>-		totem_action_set_mrl_and_play (totem, mrl);
>+		subtitle = totem_playlist_get_current_subtitle (totem->playlist);
>+		totem_action_set_mrl_and_play (totem, mrl, subtitle);

I'd rather _get_current_subtitle also fetched the subtitle, eg.
mrl = totem_playlist_get_current_mrl (totem->playlist, &subtitle);

> void
> totem_action_play_media_device (Totem *totem, const char *device)
> {
>-	char *mrl;
>+	char *mrl, *subtitle;
> 
> 	if (totem_action_load_media_device (totem, device) != FALSE) {
> 		mrl = totem_playlist_get_current_mrl (totem->playlist);
>-		totem_action_set_mrl_and_play (totem, mrl);
>+		subtitle = totem_playlist_get_current_subtitle (totem->playlist);
>+		totem_action_set_mrl_and_play (totem, mrl, subtitle);
> 		g_free (mrl);
>+		g_free (subtitle);

That's unneeded, you can't use text subtitles against mediums.

> 	}
> }
> 
> void
> totem_action_play_media (Totem *totem, TotemDiscMediaType type, const char *device)
> {

Same here.
<snip>
>+		if (!subtitle && totem->autoload_subs != FALSE)
>+			subtitle_uri = g_strdup(totem_uri_get_subtitle_uri (mrl));
>+		else
>+			subtitle_uri = g_strdup(subtitle);
>+
<snip>
>+		g_free(subtitle_uri);

Do you really need to duplicate subtitle to free it seconds afterwards?

<snip>
>diff -Naur totem-2.21.0.orig/src/totem-playlist.c totem-2.21.0/src/totem-playlist.c
>--- totem-2.21.0.orig/src/totem-playlist.c	2007-10-19 13:28:41.000000000 +0200
>+++ totem-2.21.0/src/totem-playlist.c	2007-11-22 00:33:43.000000000 +0100
>@@ -49,6 +49,7 @@
> void totem_playlist_up_files (GtkWidget *widget, TotemPlaylist *playlist);
> void totem_playlist_down_files (GtkWidget *widget, TotemPlaylist *playlist);
> void playlist_copy_location_action_callback (GtkAction *action, TotemPlaylist *playlist);
>+void playlist_select_subtitles_action_callback (GtkAction *action, TotemPlaylist *playlist);

I'd rather not have any UI for loading subtitles in the playlist widget.
Comment 55 Kamil Pawlowski 2007-11-23 11:13:40 UTC
Created attachment 99522 [details] [review]
totem graphic selection of the subtitles

I make all you suggestions and make diff against SVN version.

If you say this is ok, I make load/save playlist with subtitles
Comment 56 Bastien Nocera 2007-11-25 01:22:48 UTC
I committed this patch with the following changes:
- a lot of fixes wrt. coding style (variable names, indentation, etc.)
- add a subtitle filter (although we need to add more subtitle types to shared-mime-info before adding them to Totem, see bug #499460)
- fix the filename->uri conversion (g_strdup_printf("file://%s", filename) is completely wrong, you should have used g_filename_to_uri instead), and make sure we only select local files
- fix memleak in _set_mrl() with the handling of subtitle strings
- make sure "subtitle" is set before using it as a parameter in a lot of calls
- mark the added menu items as translatable, and use open instead of copy as the icon

There's also a few other bugs, see bug 499462, and bug 499463

Thanks very much Kamil for the patch.

2007-11-25  Bastien Nocera  <hadess@hadess.net>

        * src/plugins/mythtv/totem-mythtv.c: (create_treeview):
        Remove row_activated signal, it's already implemented in
        the list view

        * bindings/python/totem.defs:
        * data/playlist.ui:
        * src/totem-menu.c: (clear_playlist_action_callback):
        * src/totem-object.c: (totem_add_to_playlist_and_play):
        * src/totem-playlist.c: (playlist_select_subtitle_action_callback),
        (playlist_copy_location_action_callback),
        (playlist_show_popup_menu), (totem_playlist_set_reorderable),
        (update_current_from_playlist), (totem_playlist_add_files),
        (playlist_remove_files), (init_treeview),
        (totem_playlist_add_one_mrl), (totem_playlist_add_mrl),
        (totem_playlist_clear), (totem_playlist_get_current_mrl),
        (totem_playlist_get_current_title), (totem_playlist_set_previous),
        (totem_playlist_class_init):
        * src/totem-playlist.h:
        * src/totem-session.c: (totem_session_restore):
        * src/totem-uri.c: (totem_setup_file_filters),
        (totem_add_subtitle), (totem_add_files):
        * src/totem-uri.h:
        * src/totem.c: (totem_action_set_mrl_and_play),
        (totem_action_open_dialog), (totem_action_load_media),
        (totem_action_play_media_device), (totem_action_play_media),
        (totem_action_play_pause), (window_state_event_cb),
        (totem_open_location_response_cb),
        (totem_action_set_mrl_with_warning), (totem_action_set_mrl),
        (totem_action_direction), (totem_action_drop_files),
        (on_got_redirect), (totem_action_set_playlist_index),
        (totem_action_remote), (playlist_changed_cb), (current_removed_cb),
        (subtitle_changed_cb), (on_eos_event),
        (totem_action_handle_key_press), (playlist_widget_setup), (main):
        * src/totem.h:
        Add big patch by Kamil Pawlowski <kamilpe@gmail.com>, adds text
        subtitle selection in the UI via right-clicking on the playlist
        item (Closes: #165981)
Comment 57 Mårten Woxberg 2007-11-25 20:38:36 UTC
This is great progress..

What different formats are supported by Totem?
And, does Totem autoload supported formats if they are named the same as the movie file (with exception of the extension)?
Comment 58 Bastien Nocera 2007-11-25 22:02:33 UTC
(In reply to comment #57)
> This is great progress..
> 
> What different formats are supported by Totem?

Those supported by the backends.

> And, does Totem autoload supported formats if they are named the same as the
> movie file (with exception of the extension)?

Yes, if you have it set.

This isn't a support forum though, bear that in mind in the future.
Comment 59 David Prieto 2007-12-08 14:42:34 UTC
It's great that this was finally added to Totem. I find it too undiscoverable, though, would it be possible to have a menu option in "view → subtitles → load file"?

If I were a new user that's where I would look.
Comment 60 Roman Polach 2007-12-08 16:44:14 UTC
I am really happy this feature is added to Totem!
Thanks to all people who participated on this.

Would it be possible also automatically load subtitles
with not the same name but similar name only?

E.g. mplayer supports "sub-fuzziness 1" option that cause
automatically load "file_en.sub" with "file.avi" video.
I think mplayer simply loads the first subtitle file it found
with filename containing video base filename.
Comment 61 Bastien Nocera 2008-02-01 17:47:51 UTC
*** Bug 513732 has been marked as a duplicate of this bug. ***