Bug 303580

Summary: First song in Amarok playlist is repeated before moving on to third track.
Product: [Frameworks and Libraries] phonon-backend-gstreamer Reporter: PhilC <bertvanwinkle>
Component: generalAssignee: Harald Sitter <sitter>
Status: VERIFIED FIXED    
Severity: normal CC: andreas.petzold+kdebugs, b.buschinski, chgonzalezg, contact, desintegr, frederic.coiffier, giggi1999, gordon.wells, heiko, iletaitunefoisfenghuang, info, kde, kevin.kofler, linuzlover, luis.davila.pc, marius, martin.sandsmark, masscss, matej, mkyral, myriam, rdieter, romain.perier, tdfischer
Priority: HI Keywords: regression, release_blocker
Version: 4.6.1   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In: 4.6.2

Description PhilC 2012-07-15 19:56:07 UTC
With phonon-gstreamer-4.6.1, the first track in an Amarok playlist is repeated before moving on to the third track (and 4th, 5th etc.). When the first track is being played for the second time, Amarok claims it is playing the second track in the playlist.

With phonon-gstreamer-4.6.0, the playlist is played correctly.

Reproducible: Always

Steps to Reproduce:
1. Install phonon-gstreamer-4.6.1.
2. Add tracks to Amarok.
3. Press PLAY.
Actual Results:  
1. The first track is played correctly. 
2. Amarok claims to be playing track 2 but track 1 is heard again.
3. Track 3 is played correctly.

Expected Results:  
Each track in the playlist should be played once only, sequentially.

KDE 4.8.97 built from git on 14/7/12.
Amarok built from git on 15/7/12. Amarok 4.6 beta1 also tested with same results.
Comment 1 Christian (Fuchs) 2012-07-16 21:08:57 UTC
Confirmed here, also since updating to phonon-gstreamer 4.6.1, gentoo linux, x86_64,  KDE is 4.9 RC2, amarok is also a current git build, so more or less the same setup.
Comment 2 Attilio 2012-07-18 14:03:23 UTC
Confirmed here too on archlinux, x86_64, KDE 4.8.4 amarok 2.5.0 and phonon-gstreamer 4.6.1.
Comment 3 Christian González 2012-07-18 16:49:55 UTC
Same here, Fedora 17 x86_64, Amarok 2.5.90, Phonon-gstreamer 4.6.1 from kde-testing repo.

Another strange symptom: when you clean your playlist while playing some track and put some other tracks on it, sometimes Amarok continue to play tracks from
the former playlist.
Comment 4 Gordon Wells 2012-07-19 00:33:21 UTC
I'm getting this too (gentoo)

amarok 2.5.90-r1, phonon-gstreamer-4.6.1
Comment 5 Marian Kyral 2012-07-19 05:25:49 UTC
Hi,
I can confirm on all my Gentoos as well. I've installed same versions as Fuchs.
Comment 6 Marian Kyral 2012-07-19 05:54:05 UTC
Installation of phonon-gstream-4.6.0 fixed it.
Comment 7 Attilio 2012-07-19 13:51:33 UTC
(In reply to comment #6)
> Installation of phonon-gstream-4.6.0 fixed it.

Yes, I confirm that downgrading this package works. It seems a software regression.

Attilio
Comment 8 Myriam Schweingruber 2012-07-19 14:27:39 UTC
Setting status to confirmed.
Comment 9 desaparecido 2012-07-20 10:25:49 UTC
I have the same bug in archlinux x86-64 with KDE 4.9 RC2 and amarok 2.5-GIT, with gstreamer-phonon backend.  With vlc-phonon works fine
Comment 10 Matěj Laitl 2012-07-22 20:57:42 UTC
Let me see if we can work-around this on the Amarok side (we already do overly serious things in slotAboutToFinish())...
Comment 11 giggi1999 2012-07-23 16:22:17 UTC
I've the same problem with ArchLinux and phonon-gstream-4.6.1.
Comment 12 MarkusH 2012-07-24 14:54:18 UTC
*** Bug 303981 has been marked as a duplicate of this bug. ***
Comment 13 Myriam Schweingruber 2012-07-24 15:25:50 UTC
(In reply to comment #10)
> Let me see if we can work-around this on the Amarok side (we already do
> overly serious things in slotAboutToFinish())...

No way, no more workarounds in Amarok around Phonon bugs, else we will end up with the same engines stack problems we currently have. Please only fix Amarok side bugs.
Comment 14 Matěj Laitl 2012-07-24 15:47:10 UTC
(In reply to comment #13)
> (In reply to comment #10)
> > Let me see if we can work-around this on the Amarok side (we already do
> > overly serious things in slotAboutToFinish())...
> 
> No way, no more workarounds in Amarok around Phonon bugs, else we will end
> up with the same engines stack problems we currently have. Please only fix
> Amarok side bugs.

Right. I've recently discovered that this is for sure phonon-gst bug and that it cannot be worked-around by Amarok. phonon-gst convicts itself, following code connected to metaDataChanged():

void
EngineController::slotMetaDataChanged()
{
    QVariantMap meta;
    meta.insert( Meta::Field::URL, m_media.data()->currentSource().url() );
    static const QList<FieldPair> fieldPairs = QList<FieldPair>()
            << FieldPair( Phonon::ArtistMetaData, Meta::Field::ARTIST )
            << FieldPair( Phonon::AlbumMetaData, Meta::Field::ALBUM )
            << FieldPair( Phonon::TitleMetaData, Meta::Field::TITLE )
            << FieldPair( Phonon::GenreMetaData, Meta::Field::GENRE )
            << FieldPair( Phonon::TracknumberMetaData, Meta::Field::TRACKNUMBER )
            << FieldPair( Phonon::DescriptionMetaData, Meta::Field::COMMENT );
    foreach( FieldPair pair, fieldPairs )
    {
        QStringList values = m_media.data()->metaData( pair.first );
        if( !values.isEmpty() )
            meta.insert( pair.second, values.first() );
    }
    debug() << "slotMetaDataChanged(): new meta-data:" << meta;

spits the following into the debugging log:

slotMetaDataChanged(): new meta-data: QMap(("xesam:album", QVariant(QString, "As I Am") ) ( "xesam:author" ,  QVariant(QString, "Alicia Keys") ) ( "xesam:genre" ,  QVariant(QString, "R&B") ) ( "xesam:title" ,  QVariant(QString, "As I Am (Intro)") ) ( "xesam:url" ,  QVariant(QUrl, QUrl("file:///home/strohel/music/mp3/Alicia Keys/2007 As I Am/02 Go Ahead.mp3") ) ) )

But "02 Go Ahead.mp3" obviously hasn't title, "As I Am (Intro)" - that is the first track. Harald, could you please have a look at this?
Comment 15 contact 2012-07-25 13:32:58 UTC
*** Bug 304013 has been marked as a duplicate of this bug. ***
Comment 16 Harald Sitter 2012-07-26 13:18:22 UTC
Please test the 4.6 or master git branch of phonon gstreamer to validate that the issue is fixed and no other issues were created.
Comment 17 Bernd Buschinski 2012-07-26 13:41:34 UTC
with a few minute old
git master amarok+phonon-gst+phonon

this bug seems to be fixed :)
Comment 18 Rex Dieter 2012-07-26 14:02:26 UTC
4.6 branch tests out a-ok for me.
Comment 19 Harald Sitter 2012-07-26 14:27:20 UTC
Awesome.

Relevant commits:
http://commits.kde.org/phonon-gstreamer/1fc07718f898026d407fda7d489c6c099a5ac004
introduction of variable to provide the fix + ensures end of stream is not skipped if we are not gapless

http://commits.kde.org/phonon-gstreamer/70cc6e14d2f36a88a4202142fb1a723b76f9ac5d
actual fix. if we get a setnextsource from phonon while gstreamer is not waiting for a source change (i.e. it is locked with about-to-finish handling) the source change must be discarded which forces phonon to set the source regularly once we switched to stopped state (i.e. playback ended and we refused to set the next source)

semi-related:
http://commits.kde.org/phonon-gstreamer/12cdb007c036fe6c03f592b7cc5f976440cd99b9
fixes gapless as the previous code actually broke it for any track which had a frontend induced state change (namely a call to play() pause() or stop()). to avoid this we only force a gapless skip when handling is actually in progress

fixing a secondary issue:
http://commits.kde.org/phonon-gstreamer/de2922ed518bd75a9be6d1a440783079fb189614
&
http://commits.kde.org/phonon-gstreamer/f91cf43f7272a01e7af661518fcf0b0c9e9c307b
when a consumer does not handle about to finish and instead sets a new source we get additional aboutofinish callbacks that need discarding so we simply reset the skipping member bool at a later point than before returning from the cb.

All to be released as .2 soonishy.
Comment 20 desaparecido 2012-07-27 15:58:29 UTC
I builded this morning from git, in kde 4.9RC2, with Archlinux (testing repo activated), with gstreamer-phonon and the bug is still there :( .  I put my playlist, play the first track, then play again the first track but is marked like is playing second track in playlist and after play the third track normally. So in my case is not  fixed yet
Comment 21 Matěj Laitl 2012-07-27 16:07:27 UTC
(In reply to comment #20)
> I builded this morning from git, in kde 4.9RC2, with Archlinux (testing repo
> activated), with gstreamer-phonon and the bug is still there :(

desaparecido, what did you build from git? This has been fixed in the phonon-gstreamer repository. To eliminate any misunderstanding, please post output of `git describe --dirty` in your phonon-gstreamer repository clone, rebuild and install it. When Amarok is running, issue `lsof -p $(pidof amarok) | grep phonon_gstreamer` and post its output so that we know the correct library is loaded.
Comment 22 desaparecido 2012-07-29 09:07:30 UTC
(In reply to comment #21)
> (In reply to comment #20)
> > I builded this morning from git, in kde 4.9RC2, with Archlinux (testing repo
> > activated), with gstreamer-phonon and the bug is still there :(
> 
> desaparecido, what did you build from git? This has been fixed in the
> phonon-gstreamer repository. To eliminate any misunderstanding, please post
> output of `git describe --dirty` in your phonon-gstreamer repository clone,
> rebuild and install it. When Amarok is running, issue `lsof -p $(pidof
> amarok) | grep phonon_gstreamer` and post its output so that we know the
> correct library is loaded.
sorry, I've misundertood, I rebuilded amarok from git, but with phonon-gstreamer 4.6.1, now I builded from git phonon-gstreamer too and all is ok, works fine, thanks..
Comment 23 Matěj Laitl 2012-07-30 14:07:18 UTC
Git commit e1d2ea6ce3ef61af5dcd4f99db9fb407c95ccb2d by Matěj Laitl.
Committed on 24/07/2012 at 10:59.
Pushed by laitl into branch 'master'.

EngineController: don't do serious work in slotAboutToFinish()

...because slotAboutToFinish() may be called twice (or not at all) per
track by some Phonon backends (hi, vlc) - increase play count rather in
slotNewTrackPlaying() or in slotFinished(). This also needs to change
how m_currentTrack is handled, because slotNewTrackPlaying() needs to
have the old one in m_currentTrack.

Also, PlaylistActions::requestNextTrack() is changed to be a read-only
method that shouldn't change playlist state especially when there is no
next track. PlaylistActions::reflectPlaybackFinished() is introduced to
do the thing and is called from EngineController::slotFinished(), which
is a much better place for it than slotAboutToFinish().

Reporters of CCed bugs, please re-test your bug with this commit
applied, it is possible it has been resolved by this patch.
Related: bug 299890, bug 268892, bug 277197, bug 302652, bug 302240
FIXED-IN: 2.6

M  +1    -0    ChangeLog
M  +44   -35   src/EngineController.cpp
M  +17   -16   src/playlist/PlaylistActions.cpp
M  +8    -0    src/playlist/PlaylistActions.h

http://commits.kde.org/amarok/e1d2ea6ce3ef61af5dcd4f99db9fb407c95ccb2d
Comment 24 Myriam Schweingruber 2012-08-08 21:07:29 UTC
*** Bug 304793 has been marked as a duplicate of this bug. ***
Comment 25 MASSUCHETTI Jonathan 2012-08-10 17:56:55 UTC
*** Bug 304934 has been marked as a duplicate of this bug. ***