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 130426 - crossfading and gapless playback support
crossfading and gapless playback support
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: playback
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
: 130606 131061 133778 172702 335258 376825 387668 564582 (view as bug list)
Depends on: 338667
Blocks:
 
 
Reported: 2004-01-03 01:10 UTC by tgnb
Modified: 2008-12-15 09:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
somewhat rough around the edges.. (133.93 KB, patch)
2006-12-08 08:40 UTC, Jonathan Matthew
none Details | Review
updated, slightly better (137.51 KB, patch)
2006-12-13 00:53 UTC, Jonathan Matthew
none Details | Review
more fixes (138.04 KB, patch)
2006-12-13 11:48 UTC, Jonathan Matthew
none Details | Review
much improved patch (160.67 KB, patch)
2007-02-22 22:21 UTC, Jonathan Matthew
none Details | Review
more things (168.20 KB, patch)
2007-02-26 21:50 UTC, Jonathan Matthew
none Details | Review
a few more things (169.99 KB, patch)
2007-02-27 13:05 UTC, Jonathan Matthew
none Details | Review
yet more stuff (200.07 KB, patch)
2007-03-18 08:53 UTC, Jonathan Matthew
none Details | Review
fix jump-to-current behaviour (201.00 KB, patch)
2007-03-18 12:31 UTC, Jonathan Matthew
none Details | Review
Test case (861.37 KB, audio/x-wav)
2007-03-18 23:26 UTC, Alexander “weej” Jones
  Details
fix deadlock on shutdown (201.33 KB, patch)
2007-03-19 22:56 UTC, Jonathan Matthew
none Details | Review
fix tick events from old player backend (201.46 KB, patch)
2007-03-20 13:34 UTC, Jonathan Matthew
none Details | Review
fixes for gstreamer core cvs (203.06 KB, patch)
2007-03-21 22:44 UTC, Jonathan Matthew
none Details | Review
fix source change behaviour (202.74 KB, patch)
2007-03-25 09:32 UTC, Jonathan Matthew
committed Details | Review

Description tgnb 2004-01-03 01:10:45 UTC
It would be nice to have support for crossfading in between songs with
options to select the duration of the fades.
To go really nutts would be options to fade out on stop/pause and fade in
on play etc.
Comment 1 Jorge Castro 2004-01-03 01:17:14 UTC
Just as a point of reference, itunes does this as an optional config
in their preferences with a checkbox and slider.

[ ] Crossfade playback

|--------------[]------------|
0           seconds         12
Comment 2 Michael Terry 2004-01-25 23:34:50 UTC
*** Bug 130606 has been marked as a duplicate of this bug. ***
Comment 3 Luke 'povman' Worth 2004-01-26 06:28:28 UTC
The xmms crossfade plugin is excellent, it would be nice to have
support like this. I mean, like having it automatically detect
continuous tracks and a gap-killer.
Comment 4 carl johan crafoord 2004-09-13 18:55:05 UTC
does no-one want to implement this? you could probably use code from the kde
player amarok (http://amarok.kde.org/) as they've got crossfading support w/
gstreamer.

also there's a quick write-up from november -03 about it here:
http://mail.gnome.org/archives/rhythmbox-devel/2003-November/msg00055.html
Comment 5 Christophe Fergeau 2004-11-01 17:35:57 UTC
Retitling the bug, gapless playback can probably be added at the same time as
crossfading is added, this allows me to mark #131061 as a dup of this bug ;)
Comment 6 Christophe Fergeau 2004-11-01 17:36:09 UTC

*** This bug has been marked as a duplicate of 131061 ***
Comment 7 Christophe Fergeau 2004-11-01 17:36:45 UTC
*** Bug 131061 has been marked as a duplicate of this bug. ***
Comment 8 Ken Harris 2004-11-19 06:29:11 UTC
Re gapless: I have 0.8.8, and I can't hear a gap.  Well, if I turn up the volume
and really concentrate and stare at the tracker, *maybe* there's a teeny gap. 
I'm not sure.  If I'm not watching the tracker, I wouldn't be able to tell you
when the gap might be.  (If this bug report didn't exist, I wouldn't have
believed that RB played gaps.)

I'm on a 900 MHz Athlon with an IDE disk (both somewhat slow and wimpy, these
days).  The files I'm listening to (e.g., Beethoven 5 -- III-IV is attacca) are
FLACs -- that might be relevant.

I seem to recall hearing that MP3, due to the way it is designed, necessarily
has a small period of silence at the start/end of tracks.  My hunch is that
RB/GST don't actually add any gap at all, but it's the silence that exists in
the MP3 files you're playing that you're hearing.  Can anybody provide any
evidence for/against this hunch?
Comment 9 Christophe Fergeau 2004-11-19 08:13:04 UTC
Ken, you are right about mp3. About FLAC being gapless in rb, rhythmbox doesn't
do anything special to try to avoid having a gap between songs afaik, ie it
stops playing the previous song, some code runs, and it starts playing the next
one. So if you are lucky, the "some code runs" part will be fast enough to avoid
hearable gaps, if you aren't, there will be a gap ;)
Comment 10 Christophe Fergeau 2005-04-05 14:05:43 UTC
*** Bug 172702 has been marked as a duplicate of this bug. ***
Comment 11 Alexander “weej” Jones 2005-11-20 18:58:17 UTC
Yes please. I have so many mix-albums encoded in FLAC and nothing on Linux will
play them back gaplessly. Have to use foobar2000 under Windows :(
Comment 12 James "Doc" Livingston 2006-01-12 10:13:53 UTC
*** Bug 133778 has been marked as a duplicate of this bug. ***
Comment 13 Alex Lancaster 2006-03-20 21:36:03 UTC
*** Bug 335258 has been marked as a duplicate of this bug. ***
Comment 14 James "Doc" Livingston 2006-04-15 01:23:27 UTC
This is doable, it just requires someone with enough knowledge of gstreamer to have a go at it. Steps would be:

1) convert RBPlayer from playbin-based to decodebin-based

2) Add a preference for transition timing. Time T=0 is normal, T<0 would be cross-fading, T>0 mean adding a gap between tracks (for bug 326128)

3) make RBPlayer emit the EOS signal before/after the normal time, according to T. Delayed would just mean adding doing the emission in a g_timeout T seconds after we receive it frm gstreamer; crossfading would involve getting the duration (D) of the track and emitting the signal (D-T) seconds after the start of the song.

4) if close() gets called on the player when the pipeline is still playing, do a fade-out and then stop.

5) if open(uri) gets called onthe player when the pipeline is still playing, use  adder and volume elements to crossfade between the old and new frontend decodebin pipline fragments.
Comment 15 Alexander “weej” Jones 2006-04-20 23:46:04 UTC
How do you actually achieve true gapless (as in 0 samples) playback with GStreamer?

The way I've always thought of it is to decode the tracks into a single audio stream of a consistent output format. (Ideally, the output format should be user-specified in some kind of generic GStreamer preference.) Somehow, you need to pre-determine the next track that is to be played in order to guarantee a 0 sample gap. If, for example, we were to simply wait until B (for "buffer") samples before the end of input stream A in the output stream until starting to add new data to the stream, you might hit a case where the next track is unable to be decoded, either through 404, corruption or just an unsupported codec. In such a case, it then becomes a hurry to find the next candidate for playback. You may well find that you run out of time and end up with a gap.

I really don't know of a consistent and robust way to do this. Perhaps simply pre-deciding on the next track as soon as the current one starts playing, and then ensuring it will be playable right up until you start buffering it is a solution.
Comment 16 Andrew Conkling 2006-04-21 00:11:33 UTC
I'm not sure if it's helpful or not (and likely tangential to the bug) but Totem seems to do a better job.
Comment 17 beerfan 2006-07-04 20:11:14 UTC
(In reply to comment #8)
> Re gapless: I have 0.8.8, and I can't hear a gap.  Well, if I turn up the volume
> and really concentrate and stare at the tracker, *maybe* there's a teeny gap. 
> I'm not sure.  If I'm not watching the tracker, I wouldn't be able to tell you
> when the gap might be.  (If this bug report didn't exist, I wouldn't have
> believed that RB played gaps.)
> 
> I'm on a 900 MHz Athlon with an IDE disk (both somewhat slow and wimpy, these
> days).  The files I'm listening to (e.g., Beethoven 5 -- III-IV is attacca) are
> FLACs -- that might be relevant.

I am listening to flacs located on my local drive on a 1.4 GHz Athlon with RB. Some songs on this album run seamlessly together and I can assure you that there is a noticeable delay between tracks.

Supporting zero gap and cross fades would be very nice. I actually was hoping that RB could do this because I just learned that the songs on Tool's Lateralus have an alternate order and run together if played in the correct sequence :-)
Comment 18 Hidde Brugmans 2006-09-01 08:29:44 UTC
interested
Comment 19 j_greenhouse 2006-10-02 18:16:22 UTC
I've been hoping for this feature ever since I've started using Rhythmbox. 

Amarok has crossfading support for Gstreamer (and Xine). I don't think these days you can lack a feature like this.
Comment 20 René Stadler 2006-10-19 16:09:07 UTC
Trying to get gapless playback to work as a start should be a good idea (gapless as in zero time between tracks, some people define it as doing silence detection at end and start).  It cannot work out of the box, here are some thoughts about it:

As the audio sink gets the EOS event, it waits until the last buffer it sent to the sound card has been played back.  Then it posts the EOS message on its bus to inform the application that playback is completely finished.  Rhythmbox receives the message and takes the pipeline down to the READY state.  Then it possibly sets the location of the next track and sets the pipeline back to PLAYING.  While this is very fast, it is not instantaneous, so the tracks are separated by a very short, yet audible gap.

To solve this, the application logic needs to be extended a bit; it needs to differentiate between two different EOS conditions: The first one is the usual variant as described, which is indicated by an EOS message on the bus.  This is the correct time to indicate in the GUI that playback is done.

A second EOS condition needs to be introduced which can be thought of as an "EOS in the decoder".  This can be detected by installing an event probe on the sink pad of the audio sink.  The probe callback being called with an EOS event indicates that the last buffer that got pushed into the audio sink was the last one of the stream.  It is also the last piece of data (event or buffer) that the playbin sends to the sink.  At that point is the time to switch to the next track (but not directly from the callback, as it as called from the streaming thread).  This can be done by locking the state of the audio sink (which is in the PLAYING state).  Then the state of the playbin/pipeline can by set without affecting the state of the sink, which should keep playing.

If the event probe callback filters out the EOS event, the audio sink will see the two streams as one, resulting in no gap at all.  This works fine as long as the new location can be opened and decoded before the end-time of the last buffer of the previous stream is reached.  Maybe a queue element is needed before the sink to ensure that switching to the next track occurs soon enough always.

I did some experiments with rb-player-gst.c that indicate it can work this way, though I don't know if I find the time to turn this into a solution that works reliably.
Comment 21 James "Doc" Livingston 2006-10-21 03:27:14 UTC
Sounds like it should (usually) work for gapless playback. I won't work correctly if the track is on slow media which takes longer to load then we have buffered - but there isn't much we can do about.
Comment 22 Andrew Conkling 2006-10-21 03:44:10 UTC
(In reply to comment #20)
> A second EOS condition needs to be introduced which can be thought of as an
> "EOS in the decoder".  This can be detected by installing an event probe on the
> sink pad of the audio sink.  The probe callback being called with an EOS event
> indicates that the last buffer that got pushed into the audio sink was the last
> one of the stream.  It is also the last piece of data (event or buffer) that
> the playbin sends to the sink.  At that point is the time to switch to the next
> track (but not directly from the callback, as it as called from the streaming
> thread).  This can be done by locking the state of the audio sink (which is in
> the PLAYING state).  Then the state of the playbin/pipeline can by set without
> affecting the state of the sink, which should keep playing.

Pardon me for not making total sense of this, but it sounds like what you're saying is that the logic of RB should be to begin buffering the next track after it gets the last data from the currently playing one? So it'd have to know what the next song would be and then start grabbing it? That sounds like a good idea, though probably not so easy. I'm looking in from the outside; I imagine the details are hardly trivial.
Comment 23 René Stadler 2006-10-21 11:13:48 UTC
"Begin buffering the next track" isn't the right term I think.  It just starts playing the next track before the first one is really finished.  You can see the source+decoder and the sink as separate entities, with an own EOS condition for each.  By the time the decoder pushes the EOS event to the sink, at least the last buffer is not played back.  There is some amount of time until the sink will have finished playing back the last buffer in full (which marks EOS), and that time is usually enough (or can be made enough) to set source+decoder to READY, change location and get back to PLAYING, stitching the tracks seamslessy together from the point of view of the sink.

It's neither magic nor a technique with excessive overhead, it's just a bit more complex.  To work in the end, some conditions must be met though:  As James says, if the next track is on slow media, there is no way to guarantee it gets started in time.  You could try to start it up in parallel to playback as early as you want in theory though.  I also suspect there are problems if the tracks have different formats, like a different sample rate, such that the audio sink has to reconfigure the hardware.  That should not be a problem in practice, assuming that tracks belonging together in such a way that you would notice the gap always come in the same formats.

I'm not even sure that the EOS separation needs to be considered in the high level mechanics of Rhythmbox (past rb-player-gst into the shell-player or even higher), at least for gapless playback it might suffice to keep the extended behaviour confined to the playing backend.
Comment 24 René Stadler 2006-10-22 17:32:02 UTC
Unfortunately I hit an obstacle on the GStreamer side of things: My (to some extend just fine working) approach so far was to state lock the audio sink in the PLAYING state so the pipeline (playbin) can go to the READY state to pick up the new location (while playback of the end of the last track continues).  It turns out that this is improper and thus has negative side effects.  The correct solution is to not touch the audio sink and the pipeline at all but just send the source+decoder part down to READY.  This cannot be done (easily) with playbin however.

So to get gapless playback to work, one needs to modify playbin or don't use it.  Since I still believe that even crossfading can work using playbin (by use of an extra element), I would refrain from dropping it (which means to implement the necessary bits manually).  Wim Taymans has started to work on a GStreamer backend for Phonon, which accounts for crossfading in its API.  It's possible that as part of this work, an elegant way is found to do it.
Comment 25 Jonathan Matthew 2006-11-18 23:57:47 UTC
*** Bug 376825 has been marked as a duplicate of this bug. ***
Comment 26 Karl Wagener 2006-11-22 18:36:00 UTC
Another interested person...
Comment 27 Christophe Dehais 2006-11-28 13:57:31 UTC
Would something like gstnonlin be useful here ? In a timeline oriented approach, doing gapless rendering, crossfading and generally fusing tracks together should be easy.
Comment 28 René Stadler 2006-11-28 14:53:24 UTC
Christophe: You probably mean gnonlin; using it would not be very useful however.  You still cannot use playbin and you still have to fiddle around with extended playback logic, have to manually set up the cross fading etc.  Gnonlin is only of use if you need it for what it was designed for: Non-linear A/V editors.
Comment 29 Jonathan Matthew 2006-12-08 08:40:32 UTC
Created attachment 77948 [details] [review]
somewhat rough around the edges..

New player backend, extensions to the rb-player interface to allow crossfading and gapless playback (by prerolling new streams before EOS of the current stream), and some UI for configuring it all.

Many things are yet to be implemented:
- seeking
- info and buffering signals
- audio CD hooks for seeking between tracks rather than creating a new pipeline
- replaygain
- control over network buffer size
- hooks for additional sinks (visualization etc.)

And there are many known problems:
- playback position shown in the UI is reset on pause/unpause
- error reporting is largely missing
- it sometimes loses track of a stream and leaves it playing when it should be faded out
- various UI glitches on track changes
- sometimes deadlocks on shutdown

All this aside, it is actually ready for testing.  It works quite well for me.

To use the new player backend, you need to open the preferences dialog, go to the new 'playback' tab, and check the 'use crossfading backend' checkbox, then restart rhythmbox.  Once the new backend is complete and working well enough, I'll probably remove this and make it a compile-time option instead.
Comment 30 Jonathan Matthew 2006-12-08 08:45:08 UTC
I should note that gapless playback (with the crossfade time set to 0) only really works for ogg vorbis and other formats that don't pad the output stream with silence.  It won't work very well for mp3.  For ogg vorbis, it's probably not sample-perfect, but it's good enough for my ears.
Comment 31 Jonathan Matthew 2006-12-08 09:28:53 UTC
The current version of the patch will crash on startup unless the new backend is enabled.  Oops.

Run 'gconftool-2 --set /apps/rhythmbox/player/use_xfade_backend --type=boolean true' to enable it.  I'll fix this in the next version of the patch.
Comment 32 René Stadler 2006-12-08 13:04:51 UTC
Good to see some activity here.  Some things to note:

 - I changed my mind about trying to keep playbin; for an audio player it is rather easy to do things "manually".  The much greater flexibility is well worth it.

 - If we can work towards bug #345181, you get completely working ReplayGain support for free since I'm working on the needed GStreamer element.  Since RB's ReplayGain implementation never worked correctly, this is even a step forward instead of just a matter of refactoring.

 - The MP3 issue is really sad.  I wonder if people realize that ripping a mix album to MP3 breaks it.  Do other players work around the issue successfully?  If the problem would be cleanly solvable, decoders would implement a solution I think.  Since they don't, I assume we are stuck with heuristics at best.

I also put together a rough-edged backend for testing some days ago and spent some time trying to meaningfully extent the player interface.  I'll review your backend and see what I can add.
Comment 33 Christophe Fergeau 2006-12-08 13:25:35 UTC
«  - The MP3 issue is really sad.  I wonder if people realize that ripping a mix
album to MP3 breaks it.  Do other players work around the issue successfully? 
If the problem would be cleanly solvable, decoders would implement a solution I
think.  Since they don't, I assume we are stuck with heuristics at best. »

Newer ipods have gapless mp3 playback (at least apple boasts it on their website). This is done by storing the exact sample count for each file in the ipod database. I guess iTunes stores it somehow in the mp3 files when it's used to rip to mp3. I seem to remember the xing header (dummy mp3 frame added by most encoder these days) can store the exact song length (without the unnecessary padding), so maybe it could be used.
(the xing header part of this comment really needs to be verified, I could be totally wrong :p )
Comment 34 James "Doc" Livingston 2006-12-11 11:59:46 UTC
This works okay for me, but there is one small change needed to make the old backend work:

rb_player_gst_close() needs to have the if test at the top changed to "if (mp->priv->uri && uri && strcmp (mp->priv->uri, uri) == 0)" so it doesn't segfault with a NULL uri.
Comment 35 Jonathan Matthew 2006-12-13 00:53:52 UTC
Created attachment 78250 [details] [review]
updated, slightly better

fixes the old player backend, installs new glade file properly, doesn't reset playback position on unpause, fixes various deadlocks and track skipping weirdness, and adds seeking when paused (still seems a bit flaky).
Comment 36 Jonathan Matthew 2006-12-13 11:48:23 UTC
Created attachment 78279 [details] [review]
more fixes

More stream state change race condition fixes, now uses duration from db entry if the player isn't giving us one (makes things work with daap).
Comment 37 James "Doc" Livingston 2006-12-20 07:41:19 UTC
*** Bug 387668 has been marked as a duplicate of this bug. ***
Comment 38 Johannes Held 2007-01-01 20:33:37 UTC
Hello and happy new year.

I'm trying to patch my sources of rhythmbox with the newest patch for crossfading.
Version: rhythmbox-0.9.7
(I'm using ArchLinux if may need this information)

The patch is applied via 
patch -Np1 -i ../../crossfade.patch 

But I get errors while patching: 

21:31:44 [/var/abs/local/rhythmbox]
makepkg -f
==> Entering fakeroot environment
==> Making package: rhythmbox 0.9.7-1 (Mon Jan  1 21:31:47 CET 2007)
==> Checking Runtime Dependencies...
==> Checking Buildtime Dependencies...
==> Retrieving Sources...
==>     Found rhythmbox-0.9.7.tar.bz2 in build dir
==> Validating source files with MD5sums
    rhythmbox-0.9.7.tar.bz2 ... Passed
==> Extracting Sources...
==>     tar --use-compress-program=bzip2 -xf rhythmbox-0.9.7.tar.bz2
==> Removing existing pkg/ directory...
==> Starting build()...
patching file backends/gstreamer/Makefile.am
patching file backends/gstreamer/rb-player-gst.c
The next patch would create the file backends/gstreamer/rb-player-gst-xfade.c,
which already exists!  Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file backends/gstreamer/rb-player-gst-xfade.c.rej
The next patch would create the file backends/gstreamer/rb-player-gst-xfade.h,
which already exists!  Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file backends/gstreamer/rb-player-gst-xfade.h.rej
patching file backends/rb-player.c
patching file backends/rb-player.h
patching file data/glade/Makefile.am
The next patch would create the file data/glade/playback-prefs.glade,
which already exists!  Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file data/glade/playback-prefs.glade.rej
patching file data/rhythmbox.schemas
Hunk #1 succeeded at 978 (offset 11 lines).
patching file lib/rb-marshal.list
patching file lib/rb-preferences.h
Hunk #2 succeeded at 70 (offset 1 line).
patching file shell/Makefile.am
patching file shell/rb-shell-player.c
Hunk #33 FAILED at 2098.
Hunk #34 succeeded at 2290 (offset 2 lines).
Hunk #36 succeeded at 2665 (offset 2 lines).
Hunk #38 succeeded at 2898 (offset 2 lines).
Hunk #40 succeeded at 2924 (offset 2 lines).
Hunk #42 succeeded at 3205 (offset 2 lines).
1 out of 42 hunks FAILED -- saving rejects to file shell/rb-shell-player.c.rej
patching file shell/rb-shell-preferences.c
==> ERROR: Build Failed.  Aborting...
21:31:53 [/var/abs/local/rhythmbox]
Comment 39 Steve Fosdick 2007-01-12 13:04:07 UTC
Regarding there being an inherent problem with gapless playback of MP3 files, the problem as I understand it is that as MP3 divides the time-domain samples into blocks before encoding.  Whenever a track doesn't occupy a whole number of blocks the final block is silence padded at the end.

There are two solutions, one better than the other - both need to happen in the time-domain, i.e. in a player after the MP3 decoder but before the audio is handed off to the sound card.

1. Record the exact length of the track (in samples) so when processing the last block the extra silence at the end can be trimmed correctly.

2. Auto-detect silence at the end of the last block and remove it.

The first of those above is the better solution because it works gaplessly with tracks where the audio of the orignal album is continuous from one track to the next but doesn't trim off silence intentionally added to the track by the producer .

As I understand it MP3 does not record the exact track length as a matter of course, but there is support for adding an ID3 tag with the exact length that can then be used to implement gapless playback.  Apparently the MP3 encoder 'lame' knows how to add the tags (and maybe others too) and some hardware players know now to use the information so compatibility with that would be good.

Regards,
Steve.
Comment 40 Jonathan Matthew 2007-02-22 22:21:22 UTC
Created attachment 83135 [details] [review]
much improved patch

UI is somewhat better (still a bit inconsistent with regard to whether it's playing or not), seeking while playing works, buffer size is configurable, buffering signals are emitted, should behave better under load.
Comment 41 Jonathan Matthew 2007-02-26 21:50:52 UTC
Created attachment 83429 [details] [review]
more things

- actually fade in the new stream when crossfading
- don't buffer cdda:// streams
- add hook for audio cd track changes (apparently not fully working)

If anyone has a better idea than the reuse-stream thing, please let me know..
Comment 42 Jonathan Matthew 2007-02-27 13:05:52 UTC
Created attachment 83458 [details] [review]
a few more things

Fixes network buffer size configuration so the slider actually works, and fails to fix it deadlocking when the buffer size is > 256kB.  hmm.

Only emit playing-song-changed when the playing song has actually changed, and try to emit playing-changed at more appropriate times.  Fixes some inconsistency in the UI and in dbus signals.
Comment 43 Jonathan Matthew 2007-03-18 08:53:18 UTC
Created attachment 84818 [details] [review]
yet more stuff

- replaygain (totally untested)
- RBPlayerGstTee and RBPlayerGstFilter (not quite right yet, but the equalizer and visualizer plugins mostly work)
- changes to visualizer plugin to use the tee interface when the player backend doesn't use playbin
- fixes fading in of mp3s with mad (wasn't broken with flump3dec..)
- maybe some minor UI things
Comment 44 Jonathan Matthew 2007-03-18 12:31:04 UTC
Created attachment 84820 [details] [review]
fix jump-to-current behaviour

stops it from jumping to the current track whenever the playing track changes.
Comment 45 James "Doc" Livingston 2007-03-18 14:28:41 UTC
Seems to work pretty well to me, and also doesn't have a vis issue that the current backend has, of resetting the visualisation between tracks.

Looks decent enough to commit whenever you're happy with it.
Comment 46 Sven Arvidsson 2007-03-18 16:00:39 UTC
This seems to work fine, really awesome work, thank you :)

Would it be possible to have an option for fade out on pause (or should I file that as a separate request)?
Comment 47 Alexander “weej” Jones 2007-03-18 16:48:44 UTC
Good effort. A few observations on "gapless" (0.00s crossfade):

1) There is still a noticable "fart" between tracks, for me. I am using FLACs, that generally have a big phat kick drum over the first and last few samples. These play back perfectly seamlessly in Foobar2000/Windows.

2) Everything feels a bit laggy, now, with about a half-second wait before RB responds to my actions.

Get this committed, so more widespread testing can be done! :)
Comment 48 Jonathan Matthew 2007-03-18 22:23:15 UTC
(In reply to comment #47)
> 1) There is still a noticable "fart" between tracks, for me. I am using FLACs,
> that generally have a big phat kick drum over the first and last few samples.
> These play back perfectly seamlessly in Foobar2000/Windows.

I haven't done any testing with FLAC.  I can't hear any artifacts between tracks in ogg vorbis format.

> 2) Everything feels a bit laggy, now, with about a half-second wait before RB
> responds to my actions.

Which actions?  next/previous?   pause?  anything else?

At the moment, I'm thinking I'll commit this once I've convinced the filter interface to work properly.  There's still more to do after that, though.
Comment 49 Alexander “weej” Jones 2007-03-18 22:38:18 UTC
The test case for this would be to manufacture a sound file which consists of all high samples of a couple of seconds in length. Now loop this, and see if we really are "gapless".

After it begins playing, it should be silent. If there is any gap at all, you will hear two clicks as it resets to 0 between the tracks.
Comment 50 Alexander “weej” Jones 2007-03-18 23:25:22 UTC
So I made said test file, and it was absolutely flawless!

OK I will investigate what's making the fart noise between some tracks. Maybe they're not ripped quite as perfectly as I thought, or maybe GStreamer has issues with the sample lengths of some formats. I will test in Foobar2000.

Pausing/Resuming, skipping tracks, all takes about a half second to actually happen now. If I pause the audio, the UI reflects the paused state, but the audio continues for a while. When I resume, the UI says "playing", but the audio stays paused for that same duration.

Also, volume manipulations, though they don't seem to take any longer than usual.

Would be nice to get these lags down to nil.

:)
Comment 51 Alexander “weej” Jones 2007-03-18 23:26:26 UTC
Created attachment 84861 [details]
Test case

This should loop without any clicks with gapless playback.
Comment 52 Alexander “weej” Jones 2007-03-18 23:29:55 UTC
Disabling fading backend causes the elapsed timer and the progress bar to stop updating, by the way. Progress bar still functions to seek, but it doesn't move by itself.
Comment 53 Alexander “weej” Jones 2007-03-19 00:31:45 UTC
OK so I ripped one of my CDs again and it plays gaplessly in FLAC. I recall using Sound Juicer to rip my CDs the first time around, and it's obviously screwed something up. :(

I notice the Play count advance about 3 seconds before the end of the current track.

Also, CD playback is a bit broken, as it skips the last 3 seconds of any track you try to play.
Comment 54 Christophe Fergeau 2007-03-19 09:41:21 UTC
(In reply to comment #53)
> OK so I ripped one of my CDs again and it plays gaplessly in FLAC. I recall
> using Sound Juicer to rip my CDs the first time around, and it's obviously
> screwed something up. :(

Did you re-rip it with sound-juicer, or did you use a different ripper this time?
Comment 55 Jonathan Matthew 2007-03-19 22:55:26 UTC
Pause/unpause will have some latency now as we don't pause the sink but instead pause the bin for the stream we're pausing.  When pausing, data already buffered in the sink will still be played, and when unpausing, new data has to work its way through the sink's buffers before it gets played.  

I'll take a look at pausing playback by pausing the sink, but I'm not sure that's the way to go.  There isn't much we can do about latency when skipping tracks.  You can probably reduce the latency by changing buffer-time/latency-time on the sink, but that's not a generally useful solution.
Comment 56 Jonathan Matthew 2007-03-19 22:56:24 UTC
Created attachment 84922 [details] [review]
fix deadlock on shutdown
Comment 57 Jonathan Matthew 2007-03-20 13:34:36 UTC
Created attachment 84970 [details] [review]
fix tick events from old player backend
Comment 58 Jonathan Matthew 2007-03-21 22:44:55 UTC
Created attachment 85083 [details] [review]
fixes for gstreamer core cvs

The previous code for starting the sink doesn't work with gstreamer core cvs.  I've also made some attempts at improving error reporting.
Comment 59 Alexander “weej” Jones 2007-03-22 03:15:35 UTC
If you're not happy with committing to trunk, how about a branch?

This is (acceptably) going to introduce some undesirable changes in behaviour. (I've noticed things like it not auto-starting playlists when you double-click on their source, for example.) If you get it committed, we can start tracking bugs!
Comment 60 Jonathan Matthew 2007-03-25 09:23:22 UTC
I don't think creating a branch for something only one person is working on really helps.  Either everyone would have to commit everything to both trunk and the branch (which obviously sucks), or I'd have to do that myself (which still sucks), or the branch would fall behind (which still sucks as the branch would be decreasingly interesting to use).

I tend to do things like this in local darcs repositories, mostly for my own sanity.  They still follow trunk fairly closely.  I can publish these easily enough without interrupting my work flow too much.  There's a copy of the repo I'm using for this work here: http://j.kaolin.wh9.net/rhythmbox/xfade ('darcs get <url>' to make a local copy, 'darcs pull' to update it).
Comment 61 Jonathan Matthew 2007-03-25 09:32:14 UTC
Created attachment 85250 [details] [review]
fix source change behaviour

The existing implementation of rb_shell_player_get_playing_entry is right for reasons I'd forgotten about..
Comment 62 James "Doc" Livingston 2007-03-27 12:26:09 UTC
Rather than marking a branch or something crazy, I'd be happy if we just committed it to trunk. After all, people can turn it off if they encounter any nasty bugs.
Comment 63 Jonathan Matthew 2007-03-27 22:24:38 UTC
OK, I've committed it to svn.
Comment 64 Alex Lancaster 2007-03-28 00:33:59 UTC
Get a build failure with latest SVN:

 gcc -DHAVE_CONFIG_H -I. -I/home/alex/src/remote-cvs/gnome.org/rhythmbox/backends/gstreamer -I../.. -DGNOMELOCALEDIR=\"/usr/local//share/locale\" -DG_LOG_DOMAIN=\"Rhythmbox\" -I/home/alex/src/remote-cvs/gnome.org/rhythmbox -I/home/alex/src/remote-cvs/gnome.org/rhythmbox/lib -I/home/alex/src/remote-cvs/gnome.org/rhythmbox/metadata -I/home/alex/src/remote-cvs/gnome.org/rhythmbox/rhythmdb -I/home/alex/src/remote-cvs/gnome.org/rhythmbox/backends -I../../lib -DORBIT2=1 -pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/libgnomeui-2.0 -I/usr/include/libgnome-2.0 -I/usr/include/libgnomecanvas-2.0 -I/usr/include/libart-2.0 -I/usr/include/gconf/2 -I/usr/include/libbonoboui-2.0 -I/usr/include/gnome-vfs-2.0 -I/usr/lib/gnome-vfs-2.0/include -I/usr/include/gnome-keyring-1 -I/usr/include/orbit-2.0 -I/usr/include/libbonobo-2.0 -I/usr/include/bonobo-activation-2.0 -I/usr/include/libxml2 -I/usr/include/libglade-2.0 -I/usr/include/gnome-vfs-module-2.0 -pthread -I/usr/include/gstreamer-0.10 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/libxml2 -Wcomment -Wformat -Wnonnull -Wimplicit-int -Wimplicit -Wmain -Wmissing-braces -Wparentheses -Wsequence-point -Wreturn-type -Wswitch -Wtrigraphs -Wunused-function -Wunused-label -Wunused-value -Wchar-subscripts -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wcast-align -Wall -Werror -std=gnu89 -DORBIT2=1 -pthread -I/usr/include/gnome-media -I/usr/include/gconf/2 -I/usr/include/gtk-2.0 -I/usr/include/libglade-2.0 -I/usr/include/libgnome-2.0 -I/usr/include/orbit-2.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/libxml2 -I/usr/include/libbonobo-2.0 -I/usr/include/gnome-vfs-2.0 -I/usr/lib/gnome-vfs-2.0/include -I/usr/include/bonobo-activation-2.0 -g -O2 -MT rb-player-gst-xfade.lo -MD -MP -MF .deps/rb-player-gst-xfade.Tpo -c /home/alex/src/remote-cvs/gnome.org/rhythmbox/backends/gstreamer/rb-player-gst-xfade.c  -fPIC -DPIC -o .libs/rb-player-gst-xfade.o
cc1: warnings being treated as errors
/home/alex/src/remote-cvs/gnome.org/rhythmbox/backends/gstreamer/rb-player-gst-xfade.c: In function 'post_buffering_message':
/home/alex/src/remote-cvs/gnome.org/rhythmbox/backends/gstreamer/rb-player-gst-xfade.c:790: warning: implicit declaration of function 'gst_message_new_buffering'
/home/alex/src/remote-cvs/gnome.org/rhythmbox/backends/gstreamer/rb-player-gst-xfade.c:790: warning: nested extern declaration of 'gst_message_new_buffering'
/home/alex/src/remote-cvs/gnome.org/rhythmbox/backends/gstreamer/rb-player-gst-xfade.c:790: warning: assignment makes pointer from integer without a cast
make[3]: *** [rb-player-gst-xfade.lo] Error 1
make[3]: Leaving directory `/home/alex/build/rhythmbox/backends/gstreamer'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/home/alex/build/rhythmbox/backends'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/alex/build/rhythmbox'
make: *** [all] Error 2
Comment 65 ntetreau 2007-03-28 10:18:48 UTC
For me, RB refuses to play wma files when using hte crossfading engine.  I'll try to get more info, perhaps someone can confirm?
Comment 66 Jonathan Matthew 2007-03-29 11:18:46 UTC
I've added the appropriate GStreamer version checks (the crossfading player backend requires 0.10.11).

I didn't do any testing with wma files because I don't really have any.  Please open a separate bug for this.
Comment 67 ntetreau 2007-03-31 12:25:03 UTC
Opened a different bug report (http://bugzilla.gnome.org/show_bug.cgi?id=424836).  Just for the record, I am using gstreamer 0.10.12 from Ubuntu Feisty

Comment 68 Jonathan Matthew 2008-12-15 09:27:03 UTC
*** Bug 564582 has been marked as a duplicate of this bug. ***