Discussion
Revision 897071
(Back to Digest)
next


Commit Details
Bug Fixes in Multimedia
Mark Kretschmann committed changes in /trunk/extragear/multimedia/amarok/src/EngineController.cpp:
When reaching the end of the playlist, make Amarok actually show that playback has stopped.
Until now it happily continued to display "Playing Foo by Bar.."

As usual with all changes to EngineController, this patch could have subtle side effects, so please keep testing extensively.
Bug 177782: Playing isnt stopped after the end of playlist, causing scrobblin...
Diff Revision 897071

Discussion
From: Dan Meltzer Date: 15th December 2008 @ 16:10:57
On Mon, Dec 15, 2008 at 4:06 AM, Mark Kretschmann wrote:
> SVN commit 897071 by markey:
>
> When reaching the end of the playlist, make Amarok actually show that
> playback has stopped. Until now it happily continued to display "Playing
> Foo by Bar.."
>
> As usual with all changes to EngineController, this patch could have
> subtle side effects, so please keep testing extensively.

Without poking my head into EngineController.cpp ... Won't this change
prevent the last tracks statistics from updating? I'm not sure how
this change fixes the issue....
>
> BUG: 177782>

From: Mark Kretschmann Date: 15th December 2008 @ 17:42:54
On Mon, Dec 15, 2008 at 5:10 PM, Dan Meltzer
wrote:
> On Mon, Dec 15, 2008 at 4:06 AM, Mark Kretschmann wrote:
>> SVN commit 897071 by markey:
>>
>> When reaching the end of the playlist, make Amarok actually show that
>> playback has stopped. Until now it happily continued to display "Playing
>> Foo by Bar.."
>>
>> As usual with all changes to EngineController, this patch could have
>> subtle side effects, so please keep testing extensively.
>
> Without poking my head into EngineController.cpp ... Won't this change
> prevent the last tracks statistics from updating? I'm not sure how
> this change fixes the issue....

This remains to be tested. The problem was that
"m_currentTrack->finishedPlaying( 1.0 )" initiated a long chain of
things it shouldn't have been doing.

--
Mark Kretschmann
Amarok Developerwww.kde.org - amarok.kde.org

From: Maximilian Kossick Date: 16th December 2008 @ 08:22:19
On Mon, Dec 15, 2008 at 6:35 PM, Mark Kretschmann wrote:
> On Mon, Dec 15, 2008 at 5:10 PM, Dan Meltzer
> wrote:
>> On Mon, Dec 15, 2008 at 4:06 AM, Mark Kretschmann wrote:
>>> SVN commit 897071 by markey:
>>>
>>> When reaching the end of the playlist, make Amarok actually show that
>>> playback has stopped. Until now it happily continued to display "Playing
>>> Foo by Bar.."
>>>
>>> As usual with all changes to EngineController, this patch could have
>>> subtle side effects, so please keep testing extensively.
>>
>> Without poking my head into EngineController.cpp ... Won't this change
>> prevent the last tracks statistics from updating? I'm not sure how
>> this change fixes the issue....
>
> This remains to be tested. The problem was that
> "m_currentTrack->finishedPlaying( 1.0 )" initiated a long chain of
> things it shouldn't have been doing.

This is not correct. finishedPlaying updates the tracks's metadata,
and therefore requires it to notify the observers. The problem is
probably in EngineController itself, in particular these rows:

emit trackFinished();
m_currentTrack->finishedPlaying( 1.0 );
m_currentTrack = 0;

I think the correct fix is to move emit trackFinished() after
m_currentTrack. Then anything that updates the GUI to show the current
track will try to retrieve the current track, which is 0, which means
that Amarok isn't playing anything at the moment.

> --
> Mark Kretschmann
> Amarok Developer
> www.kde.org - amarok.kde.org>

From: Mark Kretschmann Date: 16th December 2008 @ 09:05:52
On Tue, Dec 16, 2008 at 9:21 AM, Maximilian Kossick
wrote:
> On Mon, Dec 15, 2008 at 6:35 PM, Mark Kretschmann wrote:
>> On Mon, Dec 15, 2008 at 5:10 PM, Dan Meltzer
>> wrote:
>>> On Mon, Dec 15, 2008 at 4:06 AM, Mark Kretschmann wrote:
>>>> SVN commit 897071 by markey:
>>>>
>>>> When reaching the end of the playlist, make Amarok actually show that
>>>> playback has stopped. Until now it happily continued to display "Playing
>>>> Foo by Bar.."
>>>>
>>>> As usual with all changes to EngineController, this patch could have
>>>> subtle side effects, so please keep testing extensively.
>>>
>>> Without poking my head into EngineController.cpp ... Won't this change
>>> prevent the last tracks statistics from updating? I'm not sure how
>>> this change fixes the issue....
>>
>> This remains to be tested. The problem was that
>> "m_currentTrack->finishedPlaying( 1.0 )" initiated a long chain of
>> things it shouldn't have been doing.
>
> This is not correct. finishedPlaying updates the tracks's metadata,
> and therefore requires it to notify the observers. The problem is
> probably in EngineController itself, in particular these rows:
>
> emit trackFinished();
> m_currentTrack->finishedPlaying( 1.0 );
> m_currentTrack = 0;
>
> I think the correct fix is to move emit trackFinished() after
> m_currentTrack. Then anything that updates the GUI to show the current
> track will try to retrieve the current track, which is 0, which means
> that Amarok isn't playing anything at the moment.

Doesn't work. Same results as before.

Either I understood you wrong, or the problem is elsewhere. Maybe you
can make a patch?

--
Mark Kretschmann
Amarok Developerwww.kde.org - amarok.kde.org

KDE Commit-Digest by Danny Allen, 2006-2009
All issues in archive by Derek Kite