Re: Review Request 109758: Asx playlist implementation.

2013-04-11 Thread Tatjana Gornak 


> On April 11, 2013, 11:58 p.m., Matěj Laitl wrote:
> > I've actually tested this patch, but it doesn't work as expected. First 
> > problem is that if I save and .asx playlist and then load it, the tracks 
> > don't load and stay grayed-out, probably because of their url is lowercased 
> > when the playlist is read. (the urls seem file inside the file) At this 
> > point I wonder why the test for .axs playlis passes just fine. Another 
> > problem is crash below.

Hm, you are right. I knew that problem, but for some reason I thought that url 
can only be case-insensitive (but, obviously I was wrong: 
http://www.w3.org/TR/WD-html40-970708/htmlweb.html)


- Tatjana


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


On April 9, 2013, 8:24 p.m., Tatjana Gornak wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109758/
> ---
> 
> (Updated April 9, 2013, 8:24 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> ---
> 
> Asx playlist implementation.
> 
> P.S. This patch make sense only if https://git.reviewboard.kde.org/r/107473/ 
> will be accepted
> 
> 
> This addresses bug 170207.
> https://bugs.kde.org/show_bug.cgi?id=170207
> 
> 
> Diffs
> -
> 
>   ChangeLog 7b394ac 
>   src/CMakeLists.txt d667d95 
>   src/MainWindow.cpp 07dca94 
>   src/core-impl/playlists/providers/user/UserPlaylistProvider.cpp e19769d 
>   src/core-impl/playlists/types/file/PlaylistFileSupport.cpp 82de3a6 
>   src/core-impl/playlists/types/file/asx/ASXPlaylist.h PRE-CREATION 
>   src/core-impl/playlists/types/file/asx/ASXPlaylist.cpp PRE-CREATION 
>   src/core/playlists/PlaylistFormat.cpp 6b3cb6b 
>   src/playlistmanager/file/PlaylistFileProvider.cpp 4a5639e 
>   tests/core-impl/playlists/types/file/CMakeLists.txt ef69236 
>   tests/core-impl/playlists/types/file/asx/TestASXPlaylist.h PRE-CREATION 
>   tests/core-impl/playlists/types/file/asx/TestASXPlaylist.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109758/diff/
> 
> 
> Testing
> ---
> 
> Loading and saving works
> 
> 
> Thanks,
> 
> Tatjana Gornak
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-11 Thread Matěj Laitl 

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


Ping.. While it may seem confusing, remarks for the latest version of the patch 
are in my comments for the version *before* the last one, sorry.

- Matěj Laitl


On March 31, 2013, 11:40 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated March 31, 2013, 11:40 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> ---
> 
> 1. Disabled Pre-amplifier in equalizer if it is not supported by phonon.
> 2. Fixed top and bottom labels of first slider. Earlier band name label was 
> at the top and slider value label was at the bottom for first slider.
> 3. Removed an extra semicolon EqualizerDialog.h .
> Note : I have made an assumption that if at all preamp is present then it 
> will the first element of Effect Parameter list.
> 
> 
> This addresses bug 301311.
> https://bugs.kde.org/show_bug.cgi?id=301311
> 
> 
> Diffs
> -
> 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 58d7360 
>   src/dialogs/EqualizerDialog.cpp 7d62e10 
>   src/dialogs/EqualizerDialog.ui 43b0187 
> 
> Diff: http://git.reviewboard.kde.org/r/108995/diff/
> 
> 
> Testing
> ---
> 
> All unit test cases passed.
> Note : I have tested it with gstreamer only. Xine phonon keep crashing on my 
> PC.
> 
> 
> File Attachments
> 
> 
> Equalizer snapshot
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/02/17/equalizer.png
> 
> 
> Thanks,
> 
> Harsh Gupta
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 109470: A patch which allows us to distinguish between cached lyrics and lyrics newly downloaded from the internet.

2013-04-11 Thread Matěj Laitl 

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


> Adds the proper fix along with the not affecting cached lyrics stuff.

I don't see it.

> Should I removenthe cached lyrics completely?

Yes, please remove the unneeded changes to cached lyrics you introduced earlier.

Also please remove the unrelated and harmful changes to CMakeLists.txt - 
commenting-out macro_optional_find_package is obviously a bad idea!

- Matěj Laitl


On April 4, 2013, 11:37 p.m., mayank jha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109470/
> ---
> 
> (Updated April 4, 2013, 11:37 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> ---
> 
> It required modifications, when there is no change in the lyrics downloaded 
> and lyrics retrieved from cache the title display of the lyrics browser 
> changes to "Cached Lyrics" from "Lyrics" so we can tell the difference 
> between old and new. 
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt d167577 
>   src/context/applets/lyrics/LyricsApplet.cpp 2394964 
>   src/context/engines/lyrics/LyricsEngine.h b187b73 
>   src/context/engines/lyrics/LyricsEngine.cpp 2befa91 
>   src/core-impl/collections/nepomukcollection/CMakeLists.txt 642919b 
> 
> Diff: http://git.reviewboard.kde.org/r/109470/diff/
> 
> 
> Testing
> ---
> 
> Its working fine!
> 
> 
> Thanks,
> 
> mayank jha
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 109817: JJ 313649 - No warning if there are no permissions to read file

2013-04-11 Thread Matěj Laitl 


> On April 12, 2013, 12:15 a.m., Matěj Laitl wrote:
> >

Oh, forgot to add general message: there was a small bit of misunderstanding 
about notPlayableReason() return type. It also should have a default 
implementation so that only relevant subclasses should be touched. Following 
subclasses can have meaningful implementations of the method:
MetaFile::Track, SqlTrack, IpodMeta::Track: specific file-based error, i.e. 
File doesn't exist/Not enough permissions to read the file/ ...
Stream: No network connection
MetaProxy::Track: "When Amarok was last closed, track with url %1 was at this 
place, but Amarok cannot find this track on filesystem or in any of your 
collections. You may try plugging in the device this track might be on."


- Matěj


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


On April 9, 2013, 1:40 p.m., Anmol Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109817/
> ---
> 
> (Updated April 9, 2013, 1:40 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> ---
> 
> 1. Added notPlayableReason() and prettyNotPlayableReason() to class Track
> 2. Display track tooltip with the prettyNotPlayableReason() if track not 
> playable
> 
> 
> Diffs
> -
> 
>   src/core-impl/collections/audiocd/AudioCdMeta.h fc26fcc 
>   src/core-impl/collections/audiocd/AudioCdMeta.cpp 218ce2a 
>   src/core-impl/collections/daap/DaapMeta.h 9a9c257 
>   src/core-impl/collections/daap/DaapMeta.cpp e66afb7 
>   src/core-impl/collections/db/sql/SqlMeta.h b7f0a71 
>   src/core-impl/collections/db/sql/SqlMeta.cpp 19ec936 
>   src/core-impl/collections/ipodcollection/IpodMeta.h 1d380b1 
>   src/core-impl/collections/ipodcollection/IpodMeta.cpp 9ffcf7e 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.h 5d1b9f9 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.cpp 2b66574 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.h 27ff06d 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.cpp 6d5bcf7 
>   src/core-impl/collections/playdarcollection/PlaydarMeta.h 7a39a4f 
>   src/core-impl/collections/playdarcollection/PlaydarMeta.cpp 5f4ec3c 
>   src/core-impl/collections/proxycollection/ProxyCollectionMeta.h ef342a4 
>   src/core-impl/collections/proxycollection/ProxyCollectionMeta.cpp e58a20a 
>   src/core-impl/collections/support/MemoryMeta.h 40061d2 
>   src/core-impl/collections/upnpcollection/UpnpMeta.h 55bc131 
>   src/core-impl/collections/upnpcollection/UpnpMeta.cpp 00cc915 
>   src/core-impl/meta/file/File.h 7d3359d 
>   src/core-impl/meta/file/File.cpp 2cd0a61 
>   src/core-impl/meta/multi/MultiTrack.h be55170 
>   src/core-impl/meta/proxy/MetaProxy.h 15967df 
>   src/core-impl/meta/proxy/MetaProxy.cpp 6a35bc5 
>   src/core-impl/meta/stream/Stream.h 3e19b45 
>   src/core-impl/meta/stream/Stream.cpp 6e35960 
>   src/core-impl/meta/timecode/TimecodeMeta.h 3aed4f9 
>   src/core-impl/meta/timecode/TimecodeMeta.cpp e3490e4 
>   src/core/meta/Meta.h f86cf39 
>   src/core/podcasts/PodcastMeta.h f3b21de 
>   src/playlist/PlaylistModel.cpp 246b9a1 
>   src/services/ServiceMetaBase.h f25159d 
>   src/services/ServiceMetaBase.cpp 7537649 
>   src/services/ampache/AmpacheMeta.h 934fe75 
>   src/services/ampache/AmpacheMeta.cpp b59715c 
>   src/services/lastfm/meta/LastFmMeta.h 9c54a10 
>   src/services/lastfm/meta/LastFmMeta.cpp 424d136 
> 
> Diff: http://git.reviewboard.kde.org/r/109817/diff/
> 
> 
> Testing
> ---
> 
> Works as expected
> 
> 
> Thanks,
> 
> Anmol Ahuja
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 109817: JJ 313649 - No warning if there are no permissions to read file

2013-04-11 Thread Matěj Laitl 

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



src/core/meta/Meta.h


Won't bee needed in next version of the patch.



src/core/meta/Meta.h


Oh, just please make the Reason a QString. (sorry for the wasted effort, I 
may have been too vague)



src/core/meta/Meta.h


Please change the documentation and implementation to:

/**
 * Returns whether playableUrl() will return a playable Url.
 * In principle this means that the url is valid.
 *
 * Default implementation returns true if notPlayableReason() is
 * empty, false otherwise.
 */



src/core/meta/Meta.h


Please make notPlayableReason() a QString and ditch 
prettyNotPlayableReason(). I imagine following docstring (yes, please provide 
default implementation):

/**
 * Return user-readable localized reason why isPlayeble() is false. 
Guaranteed to be empty if isPlayable() is true.
 *
 * Default implementation just returns an empty string.
 */

Also please don't implement methods in header files in normal cases.



src/playlist/PlaylistModel.cpp


Thinking about it more, the logic should be following:

if( s_showToolTip )
toolTipForTrack() // contains logic for showing not played notice if 
track isn't playable
else if( !track->notPlayebleReason().isEmpty() )
return i18n( "Note: the track is not playable.\n%1", 
notPlayableReason );  // i.e. just show the notice if s_showToolTip is false


- Matěj Laitl


On April 9, 2013, 1:40 p.m., Anmol Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109817/
> ---
> 
> (Updated April 9, 2013, 1:40 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> ---
> 
> 1. Added notPlayableReason() and prettyNotPlayableReason() to class Track
> 2. Display track tooltip with the prettyNotPlayableReason() if track not 
> playable
> 
> 
> Diffs
> -
> 
>   src/core-impl/collections/audiocd/AudioCdMeta.h fc26fcc 
>   src/core-impl/collections/audiocd/AudioCdMeta.cpp 218ce2a 
>   src/core-impl/collections/daap/DaapMeta.h 9a9c257 
>   src/core-impl/collections/daap/DaapMeta.cpp e66afb7 
>   src/core-impl/collections/db/sql/SqlMeta.h b7f0a71 
>   src/core-impl/collections/db/sql/SqlMeta.cpp 19ec936 
>   src/core-impl/collections/ipodcollection/IpodMeta.h 1d380b1 
>   src/core-impl/collections/ipodcollection/IpodMeta.cpp 9ffcf7e 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.h 5d1b9f9 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.cpp 2b66574 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.h 27ff06d 
>   src/core-impl/collections/nepomukcollection/meta/NepomukTrack.cpp 6d5bcf7 
>   src/core-impl/collections/playdarcollection/PlaydarMeta.h 7a39a4f 
>   src/core-impl/collections/playdarcollection/PlaydarMeta.cpp 5f4ec3c 
>   src/core-impl/collections/proxycollection/ProxyCollectionMeta.h ef342a4 
>   src/core-impl/collections/proxycollection/ProxyCollectionMeta.cpp e58a20a 
>   src/core-impl/collections/support/MemoryMeta.h 40061d2 
>   src/core-impl/collections/upnpcollection/UpnpMeta.h 55bc131 
>   src/core-impl/collections/upnpcollection/UpnpMeta.cpp 00cc915 
>   src/core-impl/meta/file/File.h 7d3359d 
>   src/core-impl/meta/file/File.cpp 2cd0a61 
>   src/core-impl/meta/multi/MultiTrack.h be55170 
>   src/core-impl/meta/proxy/MetaProxy.h 15967df 
>   src/core-impl/meta/proxy/MetaProxy.cpp 6a35bc5 
>   src/core-impl/meta/stream/Stream.h 3e19b45 
>   src/core-impl/meta/stream/Stream.cpp 6e35960 
>   src/core-impl/meta/timecode/TimecodeMeta.h 3aed4f9 
>   src/core-impl/meta/timecode/TimecodeMeta.cpp e3490e4 
>   src/core/meta/Meta.h f86cf39 
>   src/core/podcasts/PodcastMeta.h f3b21de 
>   src/playlist/PlaylistModel.cpp 246b9a1 
>   src/services/ServiceMetaBase.h f25159d 
>   src/services/ServiceMetaBase.cpp 7537649 
>   src/services/ampache/AmpacheMeta.h 934fe75 
>   src/services/ampache/AmpacheMeta.cpp b59715c 
>   src/services/lastfm/meta/LastFmMeta.h 9c54a10 
>   src/services/lastfm/meta/LastFmMeta.cpp 424d136 
> 
> Diff: http://git.reviewboard.kde.org/r/109817/diff/
> 
> 
> Testing
> ---
> 
> Works as expected
> 
> 
> Thanks,
> 
> Anmol Ahuja
> 
>

___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Re: Review Request 109758: Asx playlist implementation.

2013-04-11 Thread Matěj Laitl 

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


I've actually tested this patch, but it doesn't work as expected. First problem 
is that if I save and .asx playlist and then load it, the tracks don't load and 
stay grayed-out, probably because of their url is lowercased when the playlist 
is read. (the urls seem file inside the file) At this point I wonder why the 
test for .axs playlis passes just fine. Another problem is crash below.


src/core-impl/playlists/types/file/asx/ASXPlaylist.cpp


typo: authhor   (I assume)


When I put an .asx playlist I've saved using Export playlist as under 
.kde/share/apps/amarok/playlists/ Amarok crashes on the following run when I 
open "Playlist Files on Disk" in Saved Playlists.
Thread 1 (Thread 0x7fb4bc2cf780 (LWP 19123)):
[KCrash Handler]
#6  0x7fb4b8d66ad5 in *__GI_raise (sig=) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#7  0x7fb4b8d67f56 in *__GI_abort () at abort.c:91
#8  0x7fb4b9459374 in qt_message_output (msgType=, 
buf=) at global/qglobal.cpp:2323
#9  0x7fb4b94594ef in qt_message(QtMsgType, const char *, typedef 
__va_list_tag __va_list_tag *) (msgType=QtFatalMsg, msg=0x7fb4b95dcc38 "ASSERT: 
\"%s\" in file %s, line %d", ap=0x7fff430027f8) at global/qglobal.cpp:2369
#10 0x7fb4b945969c in qFatal (msg=) at 
global/qglobal.cpp:2552
#11 0x7fb4bb1d3bc4 in KSharedPtr::operator-> 
(this=0x7fff430028e0) at /usr/include/KDE/../ksharedptr.h:126
#12 0x7fb4bb1d35e0 in Playlists::PlaylistFile::description (this=0x2f14820) 
at 
/home/strohel/projekty/amarok/src/core-impl/playlists/types/file/PlaylistFile.cpp:155
#13 0x7fb4bb2391c6 in PlaylistBrowserNS::PlaylistBrowserModel::data 
(this=0x2bdacd0, index=..., role=91) at 
/home/strohel/projekty/amarok/src/browsers/playlistbrowser/PlaylistBrowserModel.cpp:110
#14 0x7fb4bb08ade1 in QModelIndex::data (this=, 
arole=) at /usr/include/qt4/QtCore/qabstractitemmodel.h:402
#15 0x7fb4bb241b8f in QtGroupingProxy::data (this=0x2bec920, index=..., 
role=91) at 
/home/strohel/projekty/amarok/src/browsers/playlistbrowser/QtGroupingProxy.cpp:439
#16 0x7fb4bb24b5e9 in PlaylistsByProviderProxy::data (this=0x2bec920, 
idx=..., role=91) at 
/home/strohel/projekty/amarok/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp:76
#17 0x7fb4ba5d5136 in QSortFilterProxyModel::data (this=, 
index=..., role=91) at itemviews/qsortfilterproxymodel.cpp:1735
#18 0x7fb4bb08ade1 in QModelIndex::data (this=, 
arole=) at /usr/include/qt4/QtCore/qabstractitemmodel.h:402
#19 0x7fb4bb391666 in PrettyTreeDelegate::sizeHint (this=0x2bfd8e0, 
option=..., index=...) at 
/home/strohel/projekty/amarok/src/widgets/PrettyTreeDelegate.cpp:256
#20 0x7fb4ba58af33 in QTreeView::indexRowSizeHint (this=, 
index=...) at itemviews/qtreeview.cpp:2815
#21 0x7fb4ba58b2c4 in itemHeight (item=2, this=0x2beff40) at 
itemviews/qtreeview.cpp:3265
#22 QTreeViewPrivate::itemHeight (this=0x2beff40, item=2) at 
itemviews/qtreeview.cpp:3254
#23 0x7fb4ba58e0a2 in QTreeViewPrivate::beginAnimatedOperation 
(this=0x2beff40) at itemviews/qtreeview.cpp:3037
#24 0x7fb4ba593642 in QTreeViewPrivate::expand (this=0x2beff40, item=1, 
emitSignal=true) at itemviews/qtreeview.cpp:2903
#25 0x7fb4ba594da0 in QTreeView::expand (this=0x2bef6e0, index=...) at 
itemviews/qtreeview.cpp:759
#26 0x7fb4bb390d15 in Amarok::PrettyTreeView::mouseReleaseEvent 
(this=0x2bef6e0, event=0x7fff430042f0) at 
/home/strohel/projekty/amarok/src/widgets/PrettyTreeView.cpp:192
#27 0x7fb4bb246ceb in 
PlaylistBrowserNS::PlaylistBrowserView::mouseReleaseEvent (this=0x2bef6e0, 
event=0x7fff430042f0) at 
/home/strohel/projekty/amarok/src/browsers/playlistbrowser/PlaylistBrowserView.cpp:98
#28 0x7fb4ba008c1e in QWidget::event (this=0x2bef6e0, event=0x7fff430042f0) 
at kernel/qwidget.cpp:8375
#29 0x7fb4ba414616 in QFrame::event (this=0x2bef6e0, e=0x7fff430042f0) at 
widgets/qframe.cpp:557
#30 0x7fb4ba540edb in QAbstractItemView::viewportEvent (this=0x2bef6e0, 
event=0x7fff430042f0) at itemviews/qabstractitemview.cpp:1644
#31 0x7fb4bb390b5b in Amarok::PrettyTreeView::viewportEvent 
(this=0x2bef6e0, event=0x7fff430042f0) at 
/home/strohel/projekty/amarok/src/widgets/PrettyTreeView.cpp:225
#32 0x7fb4b9571014 in 
QCoreApplicationPrivate::sendThroughObjectEventFilters (this=, 
receiver=0x2bf0ad0, event=0x7fff430042f0) at kernel/qcoreapplication.cpp:1056
#33 0x7fb4b9faf3e9 in notify_helper (e=0x7fff430042f0, receiver=0x2bf0ad0, 
this=0x1c37670) at kernel/qapplication.cpp:4558
#34 QApplicationPrivate::notify_helper (this=0x1c37670, receiver=0x2bf0ad0, 
e=0x7fff430042f0) at kernel/qapplication.cpp:4534
#35 0x7fb4b9fb581a in QApplication::notify (this=, 
receiver=0x2bf0ad0, e=0x7fff43

Re: GSoC considerations

2013-04-11 Thread Edward Toroshchin 
Hi,

I guess the first question is quite important: who is going to mentor at
all?

On Tue, Apr 09, 2013 at 03:46:31PM +0200, Matěj Laitl wrote:
> I've added 3 ideas, but I might not be able to mentor them in case I'm 
> accepted as a student, however I plan to do code reviews. Please have a look 
> at these - if they seem nonsense, please shout early! "I could mentor this" 
> or 
> "I suggest a change" are of course also accepted. ;)

They look quite good to me: well described and timely. For which one are
you going to apply yourself?

I belive I would be able to mentor either of the projects.

I didn't read into the details, though, I think we should do it together
with the students who want to apply. Those are ideas, after all, not
thorough project specs.

> I have also another idea on my mind: Scripting Interface Revival

Why revival? It's alive and kicking.

Still, this is a nice idea, but the project should start with a review
of existing scripts, API calls they use and don't use, probably contact
some of the script authors and ask what they like/dislike/want/don't
want (hey, script writers, are you reading this?).

> Myriam also suggests that we should run the "QML Context View" idea again and 
> I support it. Agreed?

No objection from me, but I don't think I'll be up to mentoring that.

I've also got an ambitious idea for the brave-hearted: a design of
Amarok 3 architecture. One should read carefully the Randa doc (scope,
vision, requirements), our codebase, and come up with the following:

 * the data model;
 * the big components layout (player engine, database, ui, ...);
 * the interaction between the components;

My personal view on this point being: the engine, media database and UI
(at least) must be separate processes, so that:

 + at least the first two can be made rock-solid, valgrinded,
   unit-tested, etc.,
 + the data structures and flows are strictly defined,
 + no bullshit a la "we can't use class XXX from YYY thread if ZZZ",
 + theoretically, any component should be completely replaceable.

I'll try to describe the benefits (as I see them) of these changes
below (if you bear with me).

 * which parts of the existing code can be spared and reused, and which
   would rather be killed with fire;
 * and so on.

I think it would be quite a challenging and interesting project.

Now, as promised, my view on the process-based architecture.

Although switching to IPC may seem over-complicated, I think it would be
actually much easier to live with than our current thread zoo.
Currently, before any tiny bit of code can be changed, a lot of things
have to be considered, which raises the entry difficulty for developers
a lot (and we're not in the best of situations there ATM).

So here are the benefits, as I see them (in a loose order of decreasing
importance):

 * it would be easier to debug and develop upon individual components
   (hence, improving the quality, userbase, and developer community),
 * database would be easy to replace (mysql, nepomuk, whatever), it
   might even become plausible once more to maintain several backends
   (although a sql and nepomuk would probably be the favorites),
 * the playback engine would do only playback, do it well, and not fuck
   up anything else (I believe currently the first question that's being
   asked in a bug report is 'have you tried changing the playback
   backend'),
 * UI might be replaceable. We might want to have a variant of an UI for
   low-performance systems. Web UI (someone mentioned recently)? Easy!
 * the environment and resource usage could be contained within each of
   the processes. Think MySQL (all the exit()'s, all the *argv, all the
   pain and misery and 500-megabyte caches for no reason).

Of course, this may not require splitting the components into processes
per se. But I still think the split is important, because it requires a
good fundamental design, and confines the development to it. Which is a
good thing.

Hope you don't regret reading all this! Cheers,

-- 
Edward "Hades" Toroshchin
dr_lepper on irc.freenode.org
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

CUE sheets in collection still not working :(

2013-04-11 Thread Myriam Schweingruber 
Hi all,

This is just a reminder for https://bugs.kde.org/show_bug.cgi?id=187587

This is a long standing bug and the one with the highest amount of votes: 751

It would be really nice if we could concentrate to solve this for the
next release, as it is really annoying.


Regards, Myriam

-- 
Proud member of the Amarok and KDE Community
Protect your freedom and join the Fellowship of FSFE:
http://www.fsfe.org
Please don't send me proprietary file formats,
use ISO standard ODF instead (ISO/IEC 26300)
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

regarding amarok development

2013-04-11 Thread Abhay Sombanshi 
hello,I am very much interested in amarok development.I want to take amarok
as my gsoc project. I find the code  little difficult to understand .I
cannot understand where to start.
I have knowledge of C++ , Qt and little bit understanding of multi
threading.Please guide me from where i can start and provide me with some
documentation or links so i could understand the code.

Thank you.
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel