| Summary: | [PATCH] phonon-gstreamer's VolumeFaderEffect doesn't react to fadeTo(), setVolume() once fading is in progress | ||
|---|---|---|---|
| Product: | [Unmaintained] phonon-backend-gstreamer | Reporter: | Matěj Laitl <matej> |
| Component: | general | Assignee: | Harald Sitter <sitter> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | myriam, romain.perier, tdfischer |
| Priority: | NOR | ||
| Version: | 4.6.2 | ||
| Target Milestone: | 4.7 | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/phonon-gstreamer/3fb51cce61f9e53fdfe857e72410445e5620107d | Version Fixed In: | 4.6.3 |
| Sentry Crash Report: | |||
| Bug Depends on: | |||
| Bug Blocks: | 312062 | ||
Forgot to note: the patch is against 4.6 branch and Amarok would be really happy if this could be included in 4.6.3. Related: Amarok bug 312062. Git commit 0d42e4933ba3ddc79a19a656378c0e6cdacc29a8 by Harald Sitter. Committed on 21/01/2013 at 17:59. Pushed by sitter into branch '4.6'. abort fades in volumefadereffect - format header to not look like the product of a drugged mind - make members private (?!) - on fadeTo and setVolume calls abort any ongoing fade (timeline::stop) - introduce inline abortFade function to reflect what is going on - introduce setVolumeInternal which is for internal use as this function is called when the timeline is running, to update the volume (no abort needed) Conflicts: gstreamer/volumefadereffect.h M +14 -2 gstreamer/volumefadereffect.cpp M +32 -26 gstreamer/volumefadereffect.h http://commits.kde.org/phonon-gstreamer/0d42e4933ba3ddc79a19a656378c0e6cdacc29a8 Git commit bb1a271e753459149f6a1383a526c296afd17737 by Harald Sitter. Committed on 21/01/2013 at 17:59. Pushed by sitter into branch 'master'. abort fades in volumefadereffect - format header to not look like the product of a drugged mind - make members private (?!) - on fadeTo and setVolume calls abort any ongoing fade (timeline::stop) - introduce inline abortFade function to reflect what is going on - introduce setVolumeInternal which is for internal use as this function is called when the timeline is running, to update the volume (no abort needed) M +14 -2 gstreamer/volumefadereffect.cpp M +32 -26 gstreamer/volumefadereffect.h http://commits.kde.org/phonon-gstreamer/bb1a271e753459149f6a1383a526c296afd17737 Git commit 3fb51cce61f9e53fdfe857e72410445e5620107d by Harald Sitter. Committed on 21/01/2013 at 17:59. Pushed by sitter into branch 'phonon4qt5'. abort fades in volumefadereffect - format header to not look like the product of a drugged mind - make members private (?!) - on fadeTo and setVolume calls abort any ongoing fade (timeline::stop) - introduce inline abortFade function to reflect what is going on - introduce setVolumeInternal which is for internal use as this function is called when the timeline is running, to update the volume (no abort needed) M +14 -2 gstreamer/volumefadereffect.cpp M +32 -26 gstreamer/volumefadereffect.h http://commits.kde.org/phonon-gstreamer/3fb51cce61f9e53fdfe857e72410445e5620107d (In reply to comment #3) > Git commit 0d42e4933ba3ddc79a19a656378c0e6cdacc29a8 by Harald Sitter. > Committed on 21/01/2013 at 17:59. > Pushed by sitter into branch '4.6'. Looks good to me, thanks. |
Steps to reproduce: 1. add VolumeFaderEffect effect to audio path, start playing a sound 2. call effect->fadeOut( 30 000 ); // 30s fade out 3. (after 10s) Oh, user wants to play another song! 4a. call effect->fadeIn( 300 ); // quick 0.3 s fade in 4b. call effect->setVolume( 1.0 ); Actual results: Both 4a and 4b don't reset currently-running fadeout, so the next song isn't audible. Also, in 4a a warning like this appears: QTimeLine: start(): cannot start when I am already running. Expected results: While the behaviour isn't documented in both cases, at least in case 4a I think it is very reasonable to abort currently-running fade, as otherwise there would be no may to do it. Proposed patch: diff --git a/gstreamer/volumefadereffect.cpp b/gstreamer/volumefadereffect.cpp index ccf51ba..424012a 100644 --- a/gstreamer/volumefadereffect.cpp +++ b/gstreamer/volumefadereffect.cpp @@ -115,6 +115,7 @@ void VolumeFaderEffect::setFadeCurve(Phonon::VolumeFaderEffect::FadeCurve pFadeC void VolumeFaderEffect::fadeTo(float targetVolume, int fadeTime) { + m_fadeTimeline->stop(); m_fadeToVolume = targetVolume; g_object_get(G_OBJECT(m_effectElement), "volume", &m_fadeFromVolume, NULL);