Bug 311193 - Switching equalizer presets from/to Inactive resets song playback
Summary: Switching equalizer presets from/to Inactive resets song playback
Status: REOPENED
Alias: None
Product: phonon-backend-gstreamer
Classification: Unclassified
Component: general (show other bugs)
Version: 4.9.0
Platform: unspecified Linux
: NOR minor (vote)
Target Milestone: ---
Assignee: Daniel Vrátil
URL:
Keywords:
: 339076 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-12-05 12:20 UTC by Germano Massullo
Modified: 2019-04-19 10:02 UTC (History)
12 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Germano Massullo 2012-12-05 12:20:25 UTC
During the playback of a song, if you switch equalizer presets from or to "Inactive", Amarok will reset song playback

Reproducible: Always

Steps to Reproduce:
1. Start the playback of a song
2. Open equalizer 
3. Switch from "Inactive" preset to another preset. Try to do the opposite.
Actual Results:  
The song playback will reset

Expected Results:  
I expect to listen to the new selected equalizer preset

Diagnostica di Amarok

Versione di Amarok: 2.6.0
Versione di KDE: 4.9.3
Versione Qt: 4.8.3
Versione di Phonon: 4.6.0
Motore di Phonon: GStreamer (4.6.2)
PulseAudio: Sì

Script di Amarok:
    Amarok Script Console 1.0 (fermato)
Lyricwiki .2 (fermato)
Free Music Charts 1.6.0 (fermato)
Librivox.org 1.0 (fermato)
Cool Streams 1.0 (fermato)

Estensioni di Amarok:
    Collezione CD audio (abilitato)
Collezione DAAP (disabilitato)
Collezione MTP (abilitato)
Collezione MySQLServer (abilitato)
Collezione MySQLe (abilitato)
Collezione UPnP (disabilitato)
Collezione iPod, iPad e iPhone (abilitato)
Collezione universale di memorizzazione di massa (abilitato)
Ampache (disabilitato)
Jamendo (disabilitato)
Last.fm (abilitato)
MP3 Music Store (disabilitato)
MP3tunes (disabilitato)
Negozio Magnatune (disabilitato)
Podcast Directory (disabilitato)
gpodder.net (disabilitato)
Motore condivisione NFS (disabilitato)
Motore condivisione SMB (Windows) (disabilitato)
Motore dei file locali e archiviazione USB di massa (abilitato)
Comment 1 Mayank Madan 2012-12-19 11:15:58 UTC
Track gets restarted when preset is changed in the equlaizer. Using v2.6.90-26-gbcdd84c
Comment 2 Myriam Schweingruber 2012-12-20 21:59:42 UTC
Confirmed by second user.
Comment 3 Ralf Engels 2013-01-24 10:48:27 UTC
This is caused by the phonon backend.

And having the equalizer always on (but with a neutral setting) is also not a solution since it seems to use up a lot of processor time.
Comment 4 Harikrishnan S 2013-02-16 19:31:38 UTC
The bug is still present. How should I go about to fix it ?
Comment 5 Mark Kretschmann 2013-08-03 09:28:04 UTC
This is not an Amarok bug. The behavior is dictated by the Phonon API.
Comment 6 John 2015-01-31 19:13:50 UTC
Please, let us not delude ourselves... This IS an Amarok bug. The behaviour is an abomination, and, sanity dictates the bug must be fixed.
Comment 7 Ralf Engels 2015-01-31 19:45:25 UTC
John, did you investigate the technical reason for this issue?

If yes, then please write some details as comment in this bug entry.
If no, then please then please be civil. It's like pointing out that cars shouldn't crash ever since it's clearly an unwanted behaviour and an abomination.

Cheers,
Ralf
Comment 8 Germano Massullo 2015-01-31 19:59:43 UTC
(In reply to John from comment #6)
> Please, let us not delude ourselves... This IS an Amarok bug. The behaviour
> is an abomination, and, sanity dictates the bug must be fixed.

No they are right, indeed this does not happen with VLC phonon backend. But they could have moved the bugreport to phonon-backend-gstreamer product instead of simply closing it. I am going to do that.
Comment 9 John 2015-01-31 21:03:44 UTC
Ralf - no, i haven't investigated anything. But it's ridiculous to say the Phonon backend dictates this behaviour.

Here is how it should work: 
  1) User presses button to enable equaliser.
  2) Song position is saved.
  3) Equalizer is enabled via whatever means necessary, 
  4) If position has been reset to start of song, then just seek to saved position & play from there.

Simple. All Amarok needs is the ability to seek to a desired position. And this is obviously a feature Amarok should always, and i mean ALWAYS, have...
Comment 10 John 2015-01-31 21:12:15 UTC
(In reply to Germano Massullo from comment #8)
> (In reply to John from comment #6)
> > Please, let us not delude ourselves... This IS an Amarok bug. The behaviour
> > is an abomination, and, sanity dictates the bug must be fixed.
> 
> No they are right, indeed this does not happen with VLC phonon backend. But
> they could have moved the bugreport to phonon-backend-gstreamer product
> instead of simply closing it. I am going to do that.

If there is some bug or feature lacking in the Phonon API, which would make this easier or cleaner to implement in Amarok, then sure, file a report against Phonon, so Amarok can work even better with future Phonon releases. 

But I still say that the bug should be fixed in Amarok. Amarok doesn't need to depend on anything special from Phonon for this.
Comment 11 Myriam Schweingruber 2015-02-01 11:05:48 UTC
(In reply to John from comment #10)
> (In reply to Germano Massullo from comment #8)
> > (In reply to John from comment #6)
> > > Please, let us not delude ourselves... This IS an Amarok bug. The behaviour
> > > is an abomination, and, sanity dictates the bug must be fixed.
> > 
> > No they are right, indeed this does not happen with VLC phonon backend. But
> > they could have moved the bugreport to phonon-backend-gstreamer product
> > instead of simply closing it. I am going to do that.
> 
> If there is some bug or feature lacking in the Phonon API, which would make
> this easier or cleaner to implement in Amarok, then sure, file a report
> against Phonon, so Amarok can work even better with future Phonon releases. 
> 
> But I still say that the bug should be fixed in Amarok. Amarok doesn't need
> to depend on anything special from Phonon for this.

And that is where you are wrong, it definitely does depend on the phonon backend, be this for searching or equalizer settings or any other playback related issues. Unless you really know the technical functions beneath Amarok and Phonon you should not issue such statements.
Comment 12 John 2015-02-01 11:57:17 UTC
(In reply to Myriam Schweingruber from comment #11)
> (In reply to John from comment #10)
> And that is where you are wrong, it definitely does depend on the phonon
> backend, be this for searching or equalizer settings or any other playback
> related issues. Unless you really know the technical functions beneath
> Amarok and Phonon you should not issue such statements.

Look. I am a software engineer, ok. I know how these things work. You are wrong.

The issue here has nothing to do with "searching for equalizer settings", or "any other playback issue". The issue is that enabling the equalizer restarts the song. 

Whatever your issues are with the equalizer and the Phonon backend, they can be overcome by seeking to the desired location in the song after the equalizer is enabled, if necessary. 

So, i repeat, to resolve this issue: "All Amarok needs is the ability to seek to a desired position. And this is obviously a feature Amarok should always, and i mean ALWAYS, have..."
Comment 13 Myriam Schweingruber 2015-02-01 12:01:32 UTC
(In reply to John from comment #12)
> (In reply to Myriam Schweingruber from comment #11)
> > (In reply to John from comment #10)
> > And that is where you are wrong, it definitely does depend on the phonon
> > backend, be this for searching or equalizer settings or any other playback
> > related issues. Unless you really know the technical functions beneath
> > Amarok and Phonon you should not issue such statements.
> 
> Look. I am a software engineer, ok. I know how these things work. You are
> wrong.
> 
> The issue here has nothing to do with "searching for equalizer settings", or
> "any other playback issue". The issue is that enabling the equalizer
> restarts the song. 
> 
> Whatever your issues are with the equalizer and the Phonon backend, they can
> be overcome by seeking to the desired location in the song after the
> equalizer is enabled, if necessary. 
> 
> So, i repeat, to resolve this issue: "All Amarok needs is the ability to
> seek to a desired position. And this is obviously a feature Amarok should
> always, and i mean ALWAYS, have..."

Again: seeking DOES depend on the backend you use, please check the code ...
Comment 14 John 2015-02-01 12:06:09 UTC
(In reply to Myriam Schweingruber from comment #11)
> (In reply to John from comment #10)
> And that is where you are wrong, it definitely does depend on the phonon
> backend, be this for searching or equalizer settings or any other playback
> related issues. Unless you really know the technical functions beneath
> Amarok and Phonon you should not issue such statements.

I would also refer you to comment #8 from Germano: "this does not happen with VLC phonon backend".

VLC, using the same backend, and the same equalizer, somehow manages to enable the equalizer without restarting the song/video.

It can be done. It must be done. It will be done. So, how about we use this bugtracker for what it is intended for eh? Which is, to track bugs and help resolve them, it is not to obfuscate with excuses until people give up in frustration.
Comment 15 John 2015-02-01 12:09:42 UTC
(In reply to Myriam Schweingruber from comment #13)
> Again: seeking DOES depend on the backend you use, please check the code ...

DOES AMAROK HAVE THE ABILITY TO USE THE PHONON BACKEND TO SEEK? 
YES, IT DOES. 
SO, USE IT.

STOP MAKING IRRELEVANT EXCUSES.
Comment 16 Myriam Schweingruber 2015-02-01 14:51:06 UTC
(In reply to John from comment #15)
> (In reply to Myriam Schweingruber from comment #13)
> > Again: seeking DOES depend on the backend you use, please check the code ...
> 
> DOES AMAROK HAVE THE ABILITY TO USE THE PHONON BACKEND TO SEEK? 
> YES, IT DOES. 
> SO, USE IT.
> 
> STOP MAKING IRRELEVANT EXCUSES.

How about you start working on your behavior and stop shouting around? Amarok will not implement different code depending on the backend the user has, that would add unnecessary complexion and simply makes no sense. Working around a faulty backend is NOT the solution. Both Phonon backends should be accessed in the same manner, so the ball is in the backend camp, the error is not on the Amarok side, as told by at least 3 Amarok team members and 2 core developers. I think they know the code better than you do, don't you think?

And instead of shouting around, how about contributing? Since you say you are a software engineer, make yourself useful and solve the issue in the backend.
Comment 17 John 2015-02-01 17:32:12 UTC
Nevermind. Just close the bug then, and someone can reopen it or create a new one, in another 2 years.

Well done. Great work. I really appreciate your commitment to producing a functional product that isn't annoying as hell.
Comment 18 John 2015-02-01 17:42:08 UTC
(In reply to Myriam Schweingruber from comment #16)
> solve the issue in the backend.

To whoever digs up this bug in 2017: 

The bug should be fixed in Amarok. Issues in the backend are irrelevant, just make it work independentgly of the backend, ie whatever the backend does.
Comment 19 Harald Sitter 2015-02-01 20:49:28 UTC
To whoever digs up this bug in 2017:

Should I not be senior software engineer working on Phonon anymore, do not pursue a workaround in Amarok but instead fix the gstreamer backend to the benefit of all software based on Phonon. Ta.
Comment 20 Daniel Vrátil 2015-02-03 15:38:03 UTC
I agree that workarounding this in Amarok is not the right way to go, but I don't see a simple way of fixing this in phonon-gstreamer either.

The phonon-gst backend intentionally resets the pipeline state before enabling/disabling the equalizer effect. According to a comment in the code it does so to prevent a deadlock in GST. Removing the line makes the backend not reset the playback (obviously), however after toggling the equalizer on and off for a while I was able to reproduce the deadlock.

The problem seems to be that while we are re-plumbing the GST pipeline from the main thread, seeking executed in another thread from our Pipeline (in reaction to a GST event) leads to an internal deadlock in GST. We could obviously protect the code by having a global mutex in phonon-gst that would prevent us from interfering with GST from multiple threads at once, but it would probably impact the performance and I'm pretty much sure we would just move the deadlocks from someone else's code into ours.

We can't do save&restore in the backend either, as the backend does not see the "big picture" of what is really happening.

I'll try to dig a bit deeper, but no promises ;)
Comment 21 John 2015-02-24 10:46:07 UTC
I see Myriam has a strong track record of closing valid and unfixed bugs without even investigating whether they are still valid. 

https://bugs.kde.org/show_bug.cgi?id=280318 - phonon-gst equalizer doesn't provide descriptive labels in Phonon::EffectParameter::name() anymore

" Myriam Schweingruber 2014-08-10 09:40:28 UTC
Since this is not about the same libraires that are current now, I close this report. Please open a new one for phonon-backend-gstreamer 4.7.2 or later, using the gstreamer 1.x libraires and plugins if this still applies."

For at least 3 1/2 years, the amarok gstreamer equalizer has had no proper frequency values on the sliders, and Myriam can't even run Amarok, and open the equalizer to see if the bug has been fixed. But she can close a perfectly valid bug.

Great work.
Comment 22 John 2015-02-24 17:28:51 UTC
Ok I have fixed this issue, with 14 lines of code.

I fixed it exactly the way that I initially proposed, ie detect if the backend resets the position in stream when equalizer is enabled or disabled, and seek back to correct position if required.

It does not involve any code that depends on the backend in any way.

Myriam, Harald, please feel free to apologise when you're ready.
Comment 23 John 2015-02-24 17:45:08 UTC
Sorry, 15 lines:
  3 lines added to EngineController.h
  4 lines added to EngineController.cpp
  8 lines added to EqualizerDialog.cpp

I could add a couple more lines to detect the very slim chance that the current track changes in the few milliseconds between grabbing the timestamp and applying the equalizer. But I think I've made my point.

Go on. Tell me again, that it depends on the backend.
Comment 24 John 2015-02-24 18:06:51 UTC
Still waiting for the apology.
No rush though.
I'm just sitting here turning my Amarok Equalizer on and off. It's fun. 
For me.

You try it though.
Go on.
Try it in your Amarok.
I doubt you'll find it so enjoyable.
Comment 25 Eike Hein 2015-02-24 18:15:24 UTC
^ The above user account has been disabled for repeated Code of Conduct violations even after a warning; nobody should feel the need wasting their time replying.
Comment 26 A_Hooman_Bean 2015-02-24 22:36:02 UTC
13 lines of code and the problem is solved:

EngineController.h :
    qint64 trackPositionMsBeforeToggle;
    bool NeedToSeekAfterEqToggled = false;
Insert above 2 lines in public section at line 60

EngineController.cpp :
      if ( NeedToSeekAfterEqToggled ) {
	NeedToSeekAfterEqToggled = false;
	seekTo(trackPositionMsBeforeToggle);
	warning() << "Backend reset track position. Had to seek back to correct track position when EQ was enabled/disabled: " << trackPositionMsBeforeToggle << " ms.";
      }
Insert above 5 lines at line 1100, just after:
    else if( newState == Phonon::PlayingState )
    {

EqualizerDialog.cpp, in modify toggleEqualizer method:
    qint64 TimeDifferenceMs;
    The::engineController()->trackPositionMsBeforeToggle = The::engineController()->trackPositionMs();
Insert above 2 lines in  at line 330, just before applyEqualizerPreset

  TimeDifferenceMs = The::engineController()->trackPositionMs() - The::engineController()->trackPositionMsBeforeToggle;
    if( TimeDifferenceMs < 0  ) { //could make comparison  vs some small number of ms, if backend might introduce just a small amount of rewind jitter on toggle.
      The::engineController()->NeedToSeekAfterEqToggled = true;
    }
Insert above 4 lines in  at line 338, just after applyEqualizerPreset

With the above fix there is still a momentary stutter when the equalizer is toggled, but the problem is fixed as well as it can be, given the gstreamer backend is so riddled with deadlocks that Phonon has to reset it.
And if the underlying problem in phonon/gstreamer is fixed at some point in the distant future, then the above code just won't be invoked.

I look forward to seeing this fix in the next Amarok release.
Comment 27 Germano Massullo 2015-05-29 13:42:57 UTC
confirming on KF 5.10
Comment 28 Myriam Schweingruber 2015-05-29 13:53:31 UTC
(In reply to Germano Massullo from comment #27)
> confirming on KF 5.10

I presume you have a newer backend version than 4.7.2, don't you? Since the gstreamer 1.x library support is only there since version 4.7.80
Pushing version to the latest released version, please correct if necessary.
Comment 29 Germano Massullo 2015-05-29 13:55:34 UTC
(In reply to Myriam Schweingruber from comment #28)
> (In reply to Germano Massullo from comment #27)
> > confirming on KF 5.10
> 
> I presume you have a newer backend version than 4.7.2, don't you? Since the
> gstreamer 1.x library support is only there since version 4.7.80
> Pushing version to the latest released version, please correct if necessary.

The bugreport is marked with 4.8.1 version but actually I have 4.8.2
Comment 30 Myriam Schweingruber 2015-05-29 14:01:49 UTC
Thank you for the fast feedback, I added the version.
Comment 31 A_Hooman_Bean 2015-10-08 14:35:56 UTC
Fast feedback on the gstreamer version is vitally important isn't it. 

For nearly 3 years, this bug has sat here, attempting to blame gstreamer, when it can be robustly fixed with 13 lines of code in Amarok.

Making a lot of progress on this bug aren't we, by taking the view that it can't be fixed in Amarok, and has to be fixed in gstreamer...

But you won't fix it in Amarok, because it'd be an admission that you were wrong.

Keep up the good work.
Comment 32 A_Hooman_Bean 2016-11-12 16:52:58 UTC
Another year has passed, i see, with no action.

The code to fix this bug has been posted.

Use it, and close the bug.
Comment 33 A_Hooman_Bean 2016-11-12 16:54:16 UTC
The code to fix this was posted 1yr, 9 months ago.
Comment 34 Myriam Schweingruber 2016-11-13 12:38:13 UTC
*** Bug 339076 has been marked as a duplicate of this bug. ***
Comment 35 Germano Massullo 2018-07-20 14:31:50 UTC
Since I don't dare telling others what they should do, after 6 years I will start searching for an Amarok replacement.
I would like also to stress out that this problem does not happen with Clementine and DeaDBeef on phonon-backend-gstreamer

Best regards
Comment 36 Rex Dieter 2018-07-20 15:03:19 UTC
In case it matters, clementine doesn't use phonon (but gstreamer directly)
Comment 37 Germano Massullo 2018-07-20 15:08:41 UTC
(In reply to Rex Dieter from comment #36)
> In case it matters, clementine doesn't use phonon (but gstreamer directly)

Thank you for the clarification, Rex
Comment 38 A_Hooman_Bean 2018-07-21 07:17:36 UTC
(In reply to Germano Massullo from comment #35)
> Since I don't dare telling others what they should do, after 6 years I will
> start searching for an Amarok replacement.
> I would like also to stress out that this problem does not happen with
> Clementine and DeaDBeef on phonon-backend-gstreamer
> 
> Best regards

Amarok is dead. It is barely usable on most systems. It just freezes for no reason. 

When you can't even adjust an equaliser without reatarting song playback, and, after 6 years, developers won't even acknowledge there is a problem, even when I've given them a fix in 14 lines of code, you know it's hopeless.

Developers don't listen. It's well past time to give up on Amarok and use anything else you can.
Comment 39 A_Hooman_Bean 2019-03-30 14:23:05 UTC
I provided 13 or 14 lines of code to fix this bug four years ago.

And now someoone has come in and marked this bug as:

depends on bug: https://bugs.kde.org/show_bug.cgi?id=406009
blocks bug: https://bugs.kde.org/show_bug.cgi?id=406010

But when i go and try to look at those bugs, it says "You are not authorized to access bug #406009." And likewise for the other one.

WHy on earth are theoe bugs inaccessible.
How is anyone supposed ot know what is going on with this bug, when it blocks/depends on other bugreports that noone can view?

This is just ridiculous.

Still, it looks like maybe we may see some progress on this bug soon!
It's not quite 7 years since the bug was reported, and only 4 years since I provided a fix, so, well done everyone!

Wonderful!
Comment 40 Christoph Feck 2019-04-19 10:02:51 UTC
It was spam, don't bother.

If you have a patch for phonon, please add it to https://phabricator.kde.org/differential/diff/create/

For more information, please read https://community.kde.org/Infrastructure/Phabricator