Review Request: Remove EngineObserver and replacing it with signals/slots for better thread safety

Ian Monroe ian.monroe at gmail.com
Sun Oct 31 16:53:10 CET 2010


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100120/
-----------------------------------------------------------

(Updated 2010-10-31 15:53:10.420749)


Review request for Amarok.


Summary (updated)
-------

Remove EngineObserver and the observer pattern.
Instead we have several new signals and functions in EngineController.
Also the EngineController is now a MetaObserver for the current track and exporting the relevant signals.

This patch improves the tread safety and reduces the complexity of the code by a lot.
Over 600 lines of codes could be removed while at the same time fixing the image update problems and several small problems where classes listened for the album meta changed but forgot that the album could change when a user changed the track.


Diffs
-----

  playground/src/context/applets/coverbling/CoverBlingApplet.h a3ff113 
  playground/src/context/applets/coverbling/CoverBlingApplet.cpp a2a50e7 
  playground/src/context/applets/video/Video.h 1d52f0b 
  playground/src/context/applets/video/Video.cpp 8471497 
  src/ActionClasses.h aa67ec7 
  src/ActionClasses.cpp 61e8af8 
  src/App.cpp e6006b9 
  src/EngineController.h aae4503 
  src/EngineController.cpp 65b3f2e 
  src/KNotificationBackend.h 02cc9d8 
  src/KNotificationBackend.cpp b98391c 
  src/MainWindow.h 4593d47 
  src/MainWindow.cpp 9102558 
  src/TrayIcon.h d1ffd43 
  src/TrayIcon.cpp 47f56bf 
  src/amarokurls/AmarokUrlHandler.cpp 1d8cd46 
  src/context/ContextView.h a3f36fb 
  src/context/ContextView.cpp ccfe4f3 
  src/context/applets/photos/PhotosApplet.h 4cac9ea 
  src/context/applets/photos/PhotosApplet.cpp 0b47da9 
  src/context/applets/similarartists/SimilarArtistsApplet.h 3a2d7ea 
  src/context/applets/similarartists/SimilarArtistsApplet.cpp 0cf2c76 
  src/context/applets/videoclip/VideoclipApplet.h a0704c6 
  src/context/applets/videoclip/VideoclipApplet.cpp 789173e 
  src/context/engines/current/CurrentEngine.h 0369065 
  src/context/engines/current/CurrentEngine.cpp 2c32f75 
  src/context/engines/labels/LabelsEngine.h 626a030 
  src/context/engines/labels/LabelsEngine.cpp 65278b4 
  src/context/engines/lyrics/LyricsEngine.h 771bf6f 
  src/context/engines/lyrics/LyricsEngine.cpp 527cd0d 
  src/context/engines/similarartists/SimilarArtistsEngine.h fea9bf5 
  src/context/engines/similarartists/SimilarArtistsEngine.cpp 97bc109 
  src/context/engines/upcomingevents/UpcomingEventsEngine.h c57b004 
  src/context/engines/upcomingevents/UpcomingEventsEngine.cpp 66131fd 
  src/context/engines/wikipedia/WikipediaEngine.h 5d53082 
  src/context/engines/wikipedia/WikipediaEngine.cpp 4c65799 
  src/core-impl/meta/stream/Stream_p.h cd217bf 
  src/core-impl/meta/timecode/TimecodeObserver.h b727196 
  src/core-impl/meta/timecode/TimecodeObserver.cpp dea809d 
  src/core/CMakeLists.txt 5863ca1 
  src/core/engine/EngineObserver.h 5a93062 
  src/core/engine/EngineObserver.cpp 7d5728b 
  src/dbus/mpris1/PlayerHandler.h 8f90f27 
  src/dbus/mpris1/PlayerHandler.cpp 0afeb59 
  src/dbus/mpris2/Mpris2DBusHandler.h a767c79 
  src/dbus/mpris2/Mpris2DBusHandler.cpp 59afbf3 
  src/dialogs/ScriptManager.h 6104dcf 
  src/dialogs/ScriptManager.cpp e98f45f 
  src/dynamic/BiasSolver.cpp bf8e3c8 
  src/dynamic/biases/EchoNest.h 9a6ecc7 
  src/dynamic/biases/EchoNest.cpp 16f26f4 
  src/mac/GrowlInterface.h 3bb35d2 
  src/mac/GrowlInterface.cpp 862d019 
  src/playlist/PlaylistActions.h 035ff77 
  src/playlist/PlaylistActions.cpp 2358b29 
  src/playlist/PlaylistController.cpp d38c9d6 
  src/playlist/view/listview/PrettyItemDelegate.cpp ab19ccc 
  src/scriptengine/AmarokEngineScript.h 55e275c 
  src/scriptengine/AmarokEngineScript.cpp 15f7b6e 
  src/services/lastfm/ScrobblerAdapter.h ef7a7c1 
  src/services/lastfm/ScrobblerAdapter.cpp 1b4da4a 
  src/services/lastfm/biases/LastFmBias.h 6ba571d 
  src/services/lastfm/biases/LastFmBias.cpp 4093104 
  src/statemanagement/DefaultApplicationController.cpp 8205aab 
  src/statusbar/StatusBar.h b785714 
  src/statusbar/StatusBar.cpp 48df821 
  src/toolbar/CurrentTrackToolbar.h c8293f3 
  src/toolbar/CurrentTrackToolbar.cpp 22408ea 
  src/toolbar/MainToolbar.h fc29ba2 
  src/toolbar/MainToolbar.cpp abeedc6 
  src/toolbar/SlimToolbar.h dcfaf22 
  src/toolbar/SlimToolbar.cpp e69df7c 
  src/toolbar/VolumePopupButton.h a0f5d56 
  src/toolbar/VolumePopupButton.cpp fb5fcb1 
  src/widgets/Osd.h b80b37e 
  src/widgets/Osd.cpp af4b24b 
  src/widgets/ProgressWidget.h 3fba490 
  src/widgets/ProgressWidget.cpp 211cea7 
  src/widgets/VolumeWidget.h 910e9f1 
  src/widgets/VolumeWidget.cpp e1d01f1 
  tests/core-impl/meta/multi/CMakeLists.txt 8a51249 
  tests/playlist/CMakeLists.txt 01703fe 

Diff: http://git.reviewboard.kde.org/r/100120/diff


Testing
-------


Thanks,

Ralf

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101031/27bac2bf/attachment.htm 


More information about the Amarok-devel mailing list