Bug 327211 - RFE: Totem should build both the xine and gstreamer backends
RFE: Totem should build both the xine and gstreamer backends
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: totem (Show other bugs)
rawhide
All Linux
low Severity low
: ---
: ---
Assigned To: Bastien Nocera
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-11 00:21 EDT by Stewart Adam
Modified: 2008-05-19 05:32 EDT (History)
3 users (show)

See Also:
Fixed In Version: 2.23.2-2.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-19 05:32:25 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
bnocera: fedora_requires_release_note+


Attachments (Terms of Use)
New totem spec file (10.72 KB, text/plain)
2007-10-11 00:21 EDT, Stewart Adam
no flags Details
Patch to add the new gconf option (613 bytes, patch)
2007-10-11 00:22 EDT, Stewart Adam
no flags Details | Diff
Script to retrieve and run the correct backend based on gconf values (378 bytes, text/plain)
2007-10-11 00:23 EDT, Stewart Adam
no flags Details
totem.spec patch (10.46 KB, patch)
2007-10-12 21:04 EDT, Stewart Adam
no flags Details | Diff
Script used to start backend-specific binaries (1.75 KB, text/plain)
2007-10-12 21:04 EDT, Stewart Adam
no flags Details
Schema patch to support "backend" key/value pair (603 bytes, patch)
2007-10-12 21:05 EDT, Stewart Adam
no flags Details | Diff
Script used to start backend-specific binaries (1.75 KB, text/plain)
2007-10-12 22:10 EDT, Stewart Adam
no flags Details
Script used to start backend-specific binaries (1.75 KB, text/plain)
2007-10-20 11:57 EDT, Stewart Adam
no flags Details
totem.spec patch for 2.21.1 (11.60 KB, patch)
2007-11-03 01:49 EDT, Stewart Adam
no flags Details | Diff
Script used to start backend-specific binaries (1.75 KB, application/x-sh)
2007-11-03 01:54 EDT, Stewart Adam
no flags Details
New patch for 2.21.95 (14.09 KB, patch)
2008-03-03 13:12 EST, Stewart Adam
no flags Details | Diff
Don't build help files (258 bytes, patch)
2008-03-03 13:12 EST, Stewart Adam
no flags Details | Diff
Fixes some translation problems, patches 2.21.95 to svn head as of 20080303 (962.58 KB, patch)
2008-03-03 13:13 EST, Stewart Adam
no flags Details | Diff
Script used to start backend-specific binaries (1.30 KB, text/plain)
2008-03-03 14:14 EST, Stewart Adam
no flags Details
The scriptlets and code needed to switch backends (1.30 KB, text/plain)
2008-03-05 15:29 EST, Stewart Adam
no flags Details
This patch adds the required 'alternatives' commands (3.02 KB, patch)
2008-03-06 11:14 EST, Stewart Adam
no flags Details | Diff
Adds the required 'alternatives' commands (3.07 KB, patch)
2008-03-06 11:51 EST, Stewart Adam
no flags Details | Diff
Adds the required 'alternatives' commands (3.87 KB, patch)
2008-03-06 15:57 EST, Stewart Adam
no flags Details | Diff
Adds the required 'alternatives' commands (revised) (3.13 KB, patch)
2008-03-13 21:52 EDT, Stewart Adam
no flags Details | Diff
totem.spec "Requires" patch (493 bytes, patch)
2008-03-20 11:26 EDT, Arkady L. Shane
no flags Details | Diff

  None (edit)
Description Stewart Adam 2007-10-11 00:21:46 EDT
This is just the result of the totem-xine review request.

Changes:
* Goes through the build twice so that both backends can be built
* Uses --program-suffix "-$BACKEND" to handle the binary files from the
different backends (Backends are placed in subpackages)
* Uses gconf to determine the backend to use at runtime

I've attached the working spec file and patches, I'll upload an SRPM tomorrow.
Comment 1 Stewart Adam 2007-10-11 00:21:46 EDT
Created attachment 223871 [details]
New totem spec file
Comment 2 Stewart Adam 2007-10-11 00:22:37 EDT
Created attachment 223881 [details]
Patch to add the new gconf option
Comment 3 Stewart Adam 2007-10-11 00:23:23 EDT
Created attachment 223891 [details]
Script to retrieve and run the correct backend based on gconf values
Comment 4 Stewart Adam 2007-10-11 00:25:34 EDT
Oh, the final recompile finished sooner than I thought. Here's the SRPM:
http://www.diffingo.com/downloads/diffingo-repo/review/totem-2.20.0-3.fc8.src.rpm
Comment 5 Bastien Nocera 2007-10-11 05:46:15 EDT
Little problem. The nautilus extension is another program that depends on a
specific backend:
%{_libdir}/nautilus/extensions-1.0/*.so*

There should only be one in there, at any one time. This is probably a good use
of alternatives. Could you please add a "--set [backend name]" to the shell
script that would set the alternatives properly (if root), and modify the gconf
value in all cases?

The Mozilla plugin doesn't seem to be switchable either:
%{_libexecdir}/totem-plugin-viewer-gstreamer
%{_libexecdir}/totem-plugin-viewer-xine

You can leave the backends in the main backend packages, and have the actual
mozilla plugins in that package.
Comment 6 Bastien Nocera 2007-10-11 05:50:26 EDT
Comment on attachment 223881 [details]
Patch to add the new gconf option

Please follow the existing code style and identation in the patch.
Comment 7 Bastien Nocera 2007-10-11 05:53:24 EDT
In the .spec:
Requires:           %{name}-gstreamer = %{version}-%{release}
Requires:           %{name}-xine = %{version}-%{release}

Add virtual Provides: in the backends, and add a Requires: totem-backend in the
main package, so people can choose to install only one backend.

Finally, please mark your text attachments as "text/plain", and attach your
.spec file changes as patches (generated with "cvs diff -up", the CVS can be
checked out as an anonymous user).
Comment 8 Stewart Adam 2007-10-12 18:43:12 EDT
Where do you suggest I store actual .so's for the nautilus plugin? (I'm assuming
that nautilus will load and .so in that directory so we can't have both .so
original folder)
Comment 9 Stewart Adam 2007-10-12 21:04:21 EDT
Created attachment 226231 [details]
totem.spec patch

Here is the new spec file
Comment 10 Stewart Adam 2007-10-12 21:04:46 EDT
Created attachment 226241 [details]
Script used to start backend-specific binaries
Comment 11 Stewart Adam 2007-10-12 21:05:16 EDT
Created attachment 226251 [details]
Schema patch to support "backend" key/value pair
Comment 12 Stewart Adam 2007-10-12 22:10:58 EDT
Created attachment 226281 [details]
Script used to start backend-specific binaries

Fixed a glitch with the totem script when using it to open files in Nautilus
Comment 13 Stewart Adam 2007-10-20 11:57:29 EDT
Created attachment 233441 [details]
Script used to start backend-specific binaries

Fixed another bug where indexing videos would make the script switch to
gstreamer. (-s is a valid switch for totem-video-thumbnailer. The wrapper
script now uses -b to switch backends)
Comment 14 Stewart Adam 2007-10-24 18:12:38 EDT
Just noticed there's missing quotes in attachment 233441 [details] around the $*, second
to last line.
Comment 15 Stewart Adam 2007-11-03 01:49:11 EDT
Created attachment 247201 [details]
totem.spec patch for 2.21.1

Updated for 2.21.1
Comment 16 Stewart Adam 2007-11-03 01:54:36 EDT
Created attachment 247211 [details]
Script used to start backend-specific binaries

New script which uses $@ rather than $* to workaround a bug, as well as
includes quotes to function properly with files including spaces.
Comment 17 Stewart Adam 2008-02-08 18:33:56 EST
Any updates on the upstream effort?
Comment 18 Stewart Adam 2008-03-03 13:12:25 EST
Created attachment 296650 [details]
New patch for 2.21.95
Comment 19 Stewart Adam 2008-03-03 13:12:56 EST
Created attachment 296651 [details]
Don't build help files
Comment 20 Stewart Adam 2008-03-03 13:13:43 EST
Created attachment 296652 [details]
Fixes some translation problems, patches 2.21.95 to svn head as of 20080303
Comment 21 Stewart Adam 2008-03-03 14:14:57 EST
Created attachment 296663 [details]
Script used to start backend-specific binaries

Now passes arguments onto totem-$backend binary
Comment 22 Bastien Nocera 2008-03-03 14:40:08 EST
Committed with 2.21.96-1
Comment 23 Hans de Goede 2008-03-04 08:11:15 EST
Good work, but those nautilus plugins really should go in a subdir of
%{_libdir}not in libdir itself, libdir itself is only for libraries.
Comment 24 Bastien Nocera 2008-03-04 19:07:45 EST
It was completely broken, package upgrade-wise, so I went a different way. Totem
now installs the backend as a shared library, instead of having it statically
linked in all the binaries.

There's a totem-backend shell script to launch totem and other binaries with a
specific backend. Eg:
totem-backend -b xine totem dvd://

If run as root, it will switch the backend for the whole system, eg.:
totem-backend -b xine
although, that's not implemented right now... Anybody willing to write the crazy
update-alternatives line?
Comment 25 Stewart Adam 2008-03-05 11:44:58 EST
Sure - So it's the same as what the script did before but I'm using alternatives
to switch the symlinks?
Comment 26 Bastien Nocera 2008-03-05 11:49:55 EST
Implementation is completely different though...

The only symlink you can switch is whether:
$(libdir)/libbaconvideowidget.so.0.0.0 points to
$(libdir)/libbaconvideowidget-gstreamer.so.0.0.0 or
$(libdir)/libbaconvideowidget-xine.so.0.0.0
Comment 27 Stewart Adam 2008-03-05 15:27:26 EST
Reopening, alternatives code attached
Comment 28 Stewart Adam 2008-03-05 15:29:22 EST
Created attachment 296927 [details]
The scriptlets and code needed to switch backends
Comment 29 Stewart Adam 2008-03-06 11:14:58 EST
Created attachment 297066 [details]
This patch adds the required 'alternatives' commands
Comment 30 Stewart Adam 2008-03-06 11:51:53 EST
Created attachment 297070 [details]
Adds the required 'alternatives' commands

Fixed a stupid typo...
Comment 31 Stewart Adam 2008-03-06 15:57:40 EST
Created attachment 297102 [details]
Adds the required 'alternatives' commands

Removed a few lines from %files and calls ldconfig in the main package's post
scriptlet (until we call ldconfig after installation, libbaconvideowidget.so.0
always point to xine).

Tested this a few times, works after every installation and the script switches
as it should.
Comment 32 Bastien Nocera 2008-03-13 20:06:19 EDT
Could you split that on multiple lines, and say something like "the xine backend
is not available":
+    echo -e 'Error: One or both of the libbaconvideowidget files not
found!\nPlease check your totem installation.'

I don't understand that:
+# works around %{_libdir}/libbaconvideowidget.so.0.0.0 pointing to xine (and
+# therefore alternatives setup is ignored)
+/sbin/ldconfig

Otherwise looks good to commit.
Comment 33 Stewart Adam 2008-03-13 21:35:36 EDT
I'm xine is build after gstreamer, so %{_libdir}/libbaconvideowidget.so.0.0.0
ends up pointing to the xine backend: at that point in time, the alternatives
don't exist so the xine library is selected by ld[config].

Once a backend has been installed and the wrapper symlink (the one we use with
alternatives) is in place, ldconfig uses that instead. So after all backends
install, we need to call a final ldconfig.

I just checked and realized the true problem - ldconfig is called before the
alternatives command in the %post and %postun scriptlets... Fixing+testing now,
if it works I'll attach a new patch.
Comment 34 Stewart Adam 2008-03-13 21:52:09 EDT
Created attachment 298004 [details]
Adds the required 'alternatives' commands (revised)

Yup, that did it. Patches backend script and spec at once, obsoleting those
patches
Comment 35 Bastien Nocera 2008-03-19 08:41:52 EDT
Looks good, please commit and kick a build.
Comment 36 Bastien Nocera 2008-03-19 09:17:46 EDT
Release notes:
---8<---
Totem, the default movie player, now has the ability to switch playback backends
without recompilation, or replacing the default package.

To change the backend system-wide:
yum install totem-xine
totem-backend -b xine

To run totem with the xine-lib backend:
yum install totem-xine
totem-backend -b xine totem
---8<---

I'm sure somebody can come up with a slightly better wording...
Comment 37 Stewart Adam 2008-03-19 11:37:03 EDT
How does this sound?
Release notes:
---8<---
Totem, the default movie player for Gnome, now has the ability to switch
playback backends without recompilation or switching packages!
To install the xine backend, run:
  yum install totem-xine

To run totem with the xine backend once, execute:
  totem-backend -b xine totem

If you'd like to change the default backend to xine for the entire system,
execute as root:
  totem-backend -b xine

Once using the xine backend, you can temporarily use the gstreamer backend in
the same manner:
  totem-backend -b gstreamer totem

Or to change the default backend back to gstreamer:
  totem-backend -b gstreamer
---8<---
Comment 38 Bastien Nocera 2008-03-19 12:52:10 EDT
Without the exclamation mark, and once you've actually committed your patch :)
Comment 39 Stewart Adam 2008-03-19 14:11:30 EDT
Done - I also changed Source0 from 2.21 to 2.23 which wasn't in the patch
original because "spectool -g" was returning 404...
Comment 40 Arkady L. Shane 2008-03-20 11:26:29 EDT
Created attachment 298705 [details]
totem.spec "Requires" patch

I think it is better to require %{name}-backend, not %{name}-gstreamer. In this
case we can remove totem-gstreamer and leave only totem-xine.
Comment 41 Bastien Nocera 2008-03-20 11:54:17 EDT
(In reply to comment #40)
> Created an attachment (id=298705) [edit]
> totem.spec "Requires" patch
> 
> I think it is better to require %{name}-backend, not %{name}-gstreamer. In this
> case we can remove totem-gstreamer and leave only totem-xine.

Not a typo. Otherwise doing a "yum install totem" will install "totem-xine"
instead of "totem-gstreamer" as its name is shorter. I blame yum...
Comment 42 Jon Stanley 2008-04-23 16:30:08 EDT
Adding FutureFeature keyword to RFE's.
Comment 43 Bastien Nocera 2008-04-23 17:54:40 EDT
(In reply to comment #42)
> Adding FutureFeature keyword to RFE's.

Did you even read the bug? It's already implemented.
Comment 44 Rahul Sundaram 2008-04-28 06:52:39 EDT
Totem in rawhide defaults to xine instead of gstreamer. We need to fix that if
we want to codeina to work. 
Comment 45 Bastien Nocera 2008-04-28 07:07:29 EDT
(In reply to comment #44)
> Totem in rawhide defaults to xine instead of gstreamer. We need to fix that if
> we want to codeina to work. 

No it doesn't. Read the scripts in the totem.spec, the GStreamer backend has a
higher priority than the xine-lib one. Must be a side-effect of upgrading from
an old version.

Note You need to log in before you can comment on or make changes to this bug.