Re: Regarding GSOC project

2013-04-15 Thread Myriam Schweingruber
Hi Abhay,

On Tue, Apr 16, 2013 at 7:35 AM, Abhay Sombanshi
 wrote:
>
> Hello,
> I hung around the amarok mailing list for a few days. I've downloaded amarok 
> from git and compiled it.
> I am focused to work upon the project "re-implementation of Amarok1.4/i-Tunes 
> import on top of statistics synchronization", in particular I would love to 
> apply for GSoC as well.
>
> So far i've understood Amarok statsyncing framework and how i-tunes stores 
> its metadata.
>
> I have worked on Qt-Creator, and have basic understanding of git and some 
> understanding of multi-threading, as well.
> My current knowledge subset pertains to C++, databases(SQL)
>
> But as dived a little deep into the code of amarok, I got a bit perturbed in 
> understanding the function of some classes.
> As there are few days left and I want to work specifically upon this project, 
> how should i proceed with the code ?
>
> Someone suggested me to solve bugs.
> Would it be a good option to start solving a bug and get a little familiarity 
> with the code ?
> If yes, then please suggest me a junior job which I can take up.

Well, that is entirely up to you which Junior Job you want to work on,
help yourself: http://tinyurl.com/amarokjjs

Also please make sure you have read all the documentation linked to
from here: http://community.kde.org/Amarok/Development/Join

Some of my fellow Amarok Team member suggested that my latest blog
post might also be a good read for aspiring GSoC students:
http://blogs.fsfe.org/myriam/2013/04/15/so-you-want-to-be-a-summer-of-code-student/


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 GSOC project

2013-04-15 Thread Abhay Sombanshi
Hello,
I hung around the amarok mailing list for a few days. I've downloaded
amarok from git and compiled it.
I am focused to work upon the project "re-implementation of
Amarok1.4/i-Tunes import on top of statistics synchronization", in
particular I would love to apply for GSoC as well.

So far i've understood Amarok statsyncing framework and how i-tunes stores
its metadata.

I have worked on Qt-Creator, and have basic understanding of git and some
understanding of multi-threading, as well.
My current knowledge subset pertains to C++, databases(SQL)

But as dived a little deep into the code of amarok, I got a bit perturbed
in understanding the function of some classes.
As there are few days left and I want to work specifically upon this
project, how should i proceed with the code ?

Someone suggested me to solve bugs.
Would it be a good option to start solving a bug and get a little
familiarity with the code ?
If yes, then please suggest me a junior job which I can take up.

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


Review Request 110036: WIP - Simple equalizer scripting

2013-04-15 Thread Ryan McCoskrie

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

Review request for Amarok.


Summary (updated)
-

WIP - Simple equalizer scripting


Description (updated)
---

A new, unambitious attempt at adding equalizer functions to the scripting 
interface.

Adds to EngineController the functions:
* QString eqPreset()
* void eqApplyPreset(QString name)
And the signal:
eqPresetApplied(QString name)

Adds to AmarokEngineScript the same functions
set up as a property.


Diffs (updated)
-

  src/EngineController.h 5de4beb 
  src/EngineController.cpp 28fb256 
  src/scriptengine/AmarokEngineScript.h f1cdb8c 
  src/scriptengine/AmarokEngineScript.cpp 4d52bbe 

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


Testing (updated)
---

Quick check using the script console that presets can be changed.
The preset does get applied but it won't show in the EqualizerDialog.
Possible other bugs.


Thanks,

Ryan McCoskrie

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


Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Alberto Villa


> On April 15, 2013, 12:54 p.m., Alberto Villa wrote:
> > src/musicbrainz/MusicBrainzTags.cpp, line 588
> > 
> >
> > Is this too hacky?
> 
> Matěj Laitl wrote:
> Calling sort( ... Qt::Descending_or_how_it_is_called ) on the view would 
> be cleaner, but we could even live with this. :-)
> 
> Matěj Laitl wrote:
> But I see you need to sort ascendingly for the "track" items. The only 
> clear solution would be to implement custom sorting method in 
> QSortFilterProxyModel to account for different sorting on different levels. 
> See StatSyncing::SortFilterProxyModel in 
> src/statsyncing/ui/MatchedTrackPage.cpp

What a pity, it was such an easy solution! :P


- Alberto


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


On April 15, 2013, 12:51 p.m., Alberto Villa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105290/
> ---
> 
> (Updated April 15, 2013, 12:51 p.m.)
> 
> 
> Review request for Amarok and Sergey Ivanov.
> 
> 
> Description
> ---
> 
> The attached patch addresses several issues and brings some improvements, 
> listed below.
> 
> Web service 2 is now being used in place of the deprecated version 1: disc 
> number and artist credit are now better defined. This alone required a 
> complete rewrite of MusicBrainzXmlParser.(cpp|h).
> 
> Make track search query more complicated and let it handle some mistakes 
> (documented in code).
> 
> As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton 
> Howard) are returned splitted (i.e., they do not have one page, but each one 
> has its own), the "Go to Artist Page" menu button was changed to a 
> KActionMenu to support showing a sub list.
> 
> Get track title from release information, as different releases can change 
> track title (e.g., adding (single edit) or similar).
> 
> Instead of doing *release* requests to get album artist and year, do *release 
> group* requests. It often means fewer requests will be done (and each one 
> takes at least 1 second, so...).
> See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
> http://tickets.musicbrainz.org/browse/SEARCH-218
> 
> Reimplement Levenshtein distance algorithm using a more efficient version 
> (pirated from Wikibooks as well as the current one).
> 
> Tracks without results are now listed and disabled, to let user know what 
> results are missing. Prior to this, you had to count them by hand.
> 
> File name is shown in track tooltip. Helpful when existing tags are equivocal.
> 
> Selected results are shown in bold font to be easily spotted.
> 
> Track number, track count, disc number (when existing) and release year 
> (first release date from the release group, actually) are now shown to help 
> distinguish among results. This means that all the fetched information is now 
> shown to the user.
> 
> Tracks are sorted by file name (fair choice). It's done after each insertion, 
> but since insertions are quite sparse, it doesn't cause any performance 
> problem.
> 
> Toolbar features "Select Best Matches" and "Deselect", the latter being new, 
> and the former being moved here from the hidden header button. To keep it not 
> too wide, Expand and Collapse buttons were modified to be KActionMenu. They 
> now require two clicks, but are easier to distinguish (I always had to stop 
> and read them to distinguish between "Collapse All" and "Collapse Chosen", 
> for example), so the time needed to click them looks mostly the same as 
> before.
> 
> The progress bar now gets to 100% even if MusicDNS search is disabled.
> 
> All releases per track are processed instead of only the best one. I'll 
> explain better. Each track is associated to several releases. In current 
> code, all the releases are parsed, but only the one with the highest score is 
> returned to the dialog. This excludes lots of good results. Now, all of them 
> can be returned. This obviously can increase the number of returned resuts, 
> but that's acceptable given that, before, you were not able to tag a lot of 
> tracks. Also, two following paragraphs are about two features that mitigate 
> this "problem".
> 
> Tracks with the same visible information are merged. There's no point in 
> showing several tracks with same title, artist, album name and artist, album 
> year, disc number, track count and track number, as the user will not be able 
> to distinguish among them. Just merge them into only one result, keeping a 
> list of artist, release and track IDs (they are and will be used). Also, the 
> score is updated to reflect the higher one. The "Go to ... Page" menu

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Matěj Laitl


> On April 15, 2013, 12:54 p.m., Alberto Villa wrote:
> > src/musicbrainz/MusicBrainzTags.cpp, line 588
> > 
> >
> > Is this too hacky?
> 
> Matěj Laitl wrote:
> Calling sort( ... Qt::Descending_or_how_it_is_called ) on the view would 
> be cleaner, but we could even live with this. :-)

But I see you need to sort ascendingly for the "track" items. The only clear 
solution would be to implement custom sorting method in QSortFilterProxyModel 
to account for different sorting on different levels. See 
StatSyncing::SortFilterProxyModel in src/statsyncing/ui/MatchedTrackPage.cpp


- Matěj


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


On April 15, 2013, 12:51 p.m., Alberto Villa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105290/
> ---
> 
> (Updated April 15, 2013, 12:51 p.m.)
> 
> 
> Review request for Amarok and Sergey Ivanov.
> 
> 
> Description
> ---
> 
> The attached patch addresses several issues and brings some improvements, 
> listed below.
> 
> Web service 2 is now being used in place of the deprecated version 1: disc 
> number and artist credit are now better defined. This alone required a 
> complete rewrite of MusicBrainzXmlParser.(cpp|h).
> 
> Make track search query more complicated and let it handle some mistakes 
> (documented in code).
> 
> As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton 
> Howard) are returned splitted (i.e., they do not have one page, but each one 
> has its own), the "Go to Artist Page" menu button was changed to a 
> KActionMenu to support showing a sub list.
> 
> Get track title from release information, as different releases can change 
> track title (e.g., adding (single edit) or similar).
> 
> Instead of doing *release* requests to get album artist and year, do *release 
> group* requests. It often means fewer requests will be done (and each one 
> takes at least 1 second, so...).
> See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
> http://tickets.musicbrainz.org/browse/SEARCH-218
> 
> Reimplement Levenshtein distance algorithm using a more efficient version 
> (pirated from Wikibooks as well as the current one).
> 
> Tracks without results are now listed and disabled, to let user know what 
> results are missing. Prior to this, you had to count them by hand.
> 
> File name is shown in track tooltip. Helpful when existing tags are equivocal.
> 
> Selected results are shown in bold font to be easily spotted.
> 
> Track number, track count, disc number (when existing) and release year 
> (first release date from the release group, actually) are now shown to help 
> distinguish among results. This means that all the fetched information is now 
> shown to the user.
> 
> Tracks are sorted by file name (fair choice). It's done after each insertion, 
> but since insertions are quite sparse, it doesn't cause any performance 
> problem.
> 
> Toolbar features "Select Best Matches" and "Deselect", the latter being new, 
> and the former being moved here from the hidden header button. To keep it not 
> too wide, Expand and Collapse buttons were modified to be KActionMenu. They 
> now require two clicks, but are easier to distinguish (I always had to stop 
> and read them to distinguish between "Collapse All" and "Collapse Chosen", 
> for example), so the time needed to click them looks mostly the same as 
> before.
> 
> The progress bar now gets to 100% even if MusicDNS search is disabled.
> 
> All releases per track are processed instead of only the best one. I'll 
> explain better. Each track is associated to several releases. In current 
> code, all the releases are parsed, but only the one with the highest score is 
> returned to the dialog. This excludes lots of good results. Now, all of them 
> can be returned. This obviously can increase the number of returned resuts, 
> but that's acceptable given that, before, you were not able to tag a lot of 
> tracks. Also, two following paragraphs are about two features that mitigate 
> this "problem".
> 
> Tracks with the same visible information are merged. There's no point in 
> showing several tracks with same title, artist, album name and artist, album 
> year, disc number, track count and track number, as the user will not be able 
> to distinguish among them. Just merge them into only one result, keeping a 
> list of artist, release and track IDs (they are and will be used). Also, the 
> score is updated to reflect the higher one. The "Go to ... Page" menu buttons 
> currently link to the top result (i.e., highest scoring) IDs, but in the 
>

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Matěj Laitl


> On April 15, 2013, 12:54 p.m., Alberto Villa wrote:
> > src/musicbrainz/MusicBrainzTags.cpp, line 588
> > 
> >
> > Is this too hacky?

Calling sort( ... Qt::Descending_or_how_it_is_called ) on the view would be 
cleaner, but we could even live with this. :-)


> On April 15, 2013, 12:54 p.m., Alberto Villa wrote:
> > src/musicbrainz/MusicBrainzTags.cpp, line 957
> > 
> >
> > I could tolerate the view knowing the specific model, but know it also 
> > needs to know if it's behind a proxy. I really don't like this approach, 
> > I'd rather prefer it to refernce only QAbstractItemModel like in other 
> > methods. I could move isChcked() and checkAllFromRelease() to the view 
> > (making them a bit more complex), but I'm not sure they belong there.
> > 
> > Any advice?

> I could tolerate the view knowing the specific model, but know it also needs 
> to know if it's behind a proxy. I really don't like this approach, I'd rather 
> prefer it to refernce only QAbstractItemModel like in other methods. I could 
> move isChcked() and checkAllFromRelease() to the view (making them a bit more 
> complex), but I'm not sure they belong there.

IMO the model is the right place.

> Any advice?

The "correct" approach would have been to provide additional role, let's call 
it "CheckedRole" (or perhaps ChoosenRole?) in MusicBrainzTagsModel and 
implement the setData() method to allow changing it through standard 
interfaces. This works fine until you start actually filtering in 
QSortFilterProxy model, where it would skip filtered items.

There seems to be no plausible solution. Perhaps the view could have a 
m_sourceModel attribute, but I leave it up to you; feel free to retain the 
current solution, it's not that bad.


- Matěj


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


On April 15, 2013, 12:51 p.m., Alberto Villa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105290/
> ---
> 
> (Updated April 15, 2013, 12:51 p.m.)
> 
> 
> Review request for Amarok and Sergey Ivanov.
> 
> 
> Description
> ---
> 
> The attached patch addresses several issues and brings some improvements, 
> listed below.
> 
> Web service 2 is now being used in place of the deprecated version 1: disc 
> number and artist credit are now better defined. This alone required a 
> complete rewrite of MusicBrainzXmlParser.(cpp|h).
> 
> Make track search query more complicated and let it handle some mistakes 
> (documented in code).
> 
> As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton 
> Howard) are returned splitted (i.e., they do not have one page, but each one 
> has its own), the "Go to Artist Page" menu button was changed to a 
> KActionMenu to support showing a sub list.
> 
> Get track title from release information, as different releases can change 
> track title (e.g., adding (single edit) or similar).
> 
> Instead of doing *release* requests to get album artist and year, do *release 
> group* requests. It often means fewer requests will be done (and each one 
> takes at least 1 second, so...).
> See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
> http://tickets.musicbrainz.org/browse/SEARCH-218
> 
> Reimplement Levenshtein distance algorithm using a more efficient version 
> (pirated from Wikibooks as well as the current one).
> 
> Tracks without results are now listed and disabled, to let user know what 
> results are missing. Prior to this, you had to count them by hand.
> 
> File name is shown in track tooltip. Helpful when existing tags are equivocal.
> 
> Selected results are shown in bold font to be easily spotted.
> 
> Track number, track count, disc number (when existing) and release year 
> (first release date from the release group, actually) are now shown to help 
> distinguish among results. This means that all the fetched information is now 
> shown to the user.
> 
> Tracks are sorted by file name (fair choice). It's done after each insertion, 
> but since insertions are quite sparse, it doesn't cause any performance 
> problem.
> 
> Toolbar features "Select Best Matches" and "Deselect", the latter being new, 
> and the former being moved here from the hidden header button. To keep it not 
> too wide, Expand and Collapse buttons were modified to be KActionMenu. They 
> now require two clicks, but are easier to distinguish (I always had to stop 
> and read them to distinguish between "Collapse All" and "Collapse Chosen", 
> for example), so the time needed to click them 

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Alberto Villa


> On April 15, 2013, 10:47 p.m., Matěj Laitl wrote:
> > Thanks for the update, glad to see it. I'm just trying out the code, 
> > however I got a deadlock - I just clicked Expand Unchosen after or during 
> > the lookup, backtrace:
> > #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:39
> > #1  0x7ff6f7528b1b in _q_futex (val2=0, addr2=0x0, timeout=0x0, val=2, 
> > op=0, addr=0x40481f0) at thread/qmutex_unix.cpp:99
> > #2  QMutexPrivate::wait (this=0x40481f0, timeout=) at 
> > thread/qmutex_unix.cpp:113
> > #3  0x7ff6f752490d in QMutex::lockInternal (this=) at 
> > thread/qmutex.cpp:450
> > #4  0x7ff6f75257ea in lockInline (this=0x4081470) at thread/qmutex.h:198
> > #5  QMutexLocker (m=0x4081470, this=0x7fff238ff4f0) at thread/qmutex.h:109
> > #6  QReadWriteLock::lockForRead (this=0x40815c8) at 
> > thread/qreadwritelock.cpp:149
> > #7  0x7ff6f91ea94d in QReadLocker::relock (this=0x7fff238ff570) at 
> > /usr/include/qt4/QtCore/qreadwritelock.h:111
> > #8  0x7ff6f9437259 in MusicBrainzTagsItem::isChecked (this=0x40815a0) 
> > at /home/strohel/projekty/amarok/src/musicbrainz/MusicBrainzTags.cpp:392
> > #9  0x7ff6f943750d in MusicBrainzTagsView::expandUnchecked 
> > (this=0x493a8c0) at 
> > /home/strohel/projekty/amarok/src/musicbrainz/MusicBrainzTags.cpp:945
> > #10 0x7ff6f764ccb7 in QMetaObject::activate (sender=0x49183e0, 
> > m=, local_signal_index=, argv=0x7fff238ff7c0)
> > at kernel/qobject.cpp:3539
> > #11 0x7ff6f806e962 in QAction::triggered (this=, 
> > _t1=false) at .moc/debug-shared/moc_qaction.cpp:277
> > #12 0x7ff6f806eb4f in QAction::activate (this=0x49183e0, 
> > event=) at kernel/qaction.cpp:1257
> > #13 0x7ff6f8492b8a in QAbstractButtonPrivate::click (this=0x491dcd0) at 
> > widgets/qabstractbutton.cpp:530
> > #14 0x7ff6f8492e3c in QAbstractButton::mouseReleaseEvent 
> > (this=0x491dca0, e=0x7fff239002b0) at widgets/qabstractbutton.cpp:1123
> > #15 0x7ff6f856209a in QToolButton::mouseReleaseEvent (this= > out>, e=) at widgets/qtoolbutton.cpp:718
> > #16 0x7ff6f80cec1e in QWidget::event (this=0x491dca0, 
> > event=0x7fff239002b0) at kernel/qwidget.cpp:8375
> > #17 0x7ff6f8075402 in notify_helper (e=0x7fff239002b0, 
> > receiver=0x491dca0, this=0x11cf820) at kernel/qapplication.cpp:4562
> > #18 QApplicationPrivate::notify_helper (this=0x11cf820, receiver=0x491dca0, 
> > e=0x7fff239002b0) at kernel/qapplication.cpp:4534
> > #19 0x7ff6f807b81a in QApplication::notify (this=, 
> > receiver=0x491dca0, e=0x7fff239002b0) at kernel/qapplication.cpp:4105
> > #20 0x7ff6f9db51a6 in KApplication::notify (this=0x7fff23900e10, 
> > receiver=0x491dca0, event=0x7fff239002b0)
> > at 
> > /var/tmp/portage/kde-base/kdelibs-4.10.1-r1/work/kdelibs-4.10.1/kdeui/kernel/kapplication.cpp:311
> > 
> > Please have a look at this.
> > 
> > When I tried the Expand Unchosen and Collapse Chosen buttons, they didn't 
> > seem to work as expected (but I tested just quickly). This is not yet a 
> > complete review, but should be a good start for further work. Anyways, 
> > thanks for the *massive* changes!

It's not supposed to work, as I stopped the porting to the proxy model to ask 
for that suggestion. I thought it wasn't even buildable. ;)


- Alberto


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


On April 15, 2013, 12:51 p.m., Alberto Villa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105290/
> ---
> 
> (Updated April 15, 2013, 12:51 p.m.)
> 
> 
> Review request for Amarok and Sergey Ivanov.
> 
> 
> Description
> ---
> 
> The attached patch addresses several issues and brings some improvements, 
> listed below.
> 
> Web service 2 is now being used in place of the deprecated version 1: disc 
> number and artist credit are now better defined. This alone required a 
> complete rewrite of MusicBrainzXmlParser.(cpp|h).
> 
> Make track search query more complicated and let it handle some mistakes 
> (documented in code).
> 
> As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton 
> Howard) are returned splitted (i.e., they do not have one page, but each one 
> has its own), the "Go to Artist Page" menu button was changed to a 
> KActionMenu to support showing a sub list.
> 
> Get track title from release information, as different releases can change 
> track title (e.g., adding (single edit) or similar).
> 
> Instead of doing *release* requests to get album artist and year, do *release 
> group* requests. It often means fewer requests will be done (and each one 
> takes at least 1 second, so...).
> See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
> h

Jenkins build became unstable: amarok_master #344

2013-04-15 Thread KDE CI System
See 

___
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-15 Thread Commit Hook

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

(Updated April 15, 2013, 9:27 p.m.)


Status
--

This change has been marked as submitted.


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 ef6790f 
  src/CMakeLists.txt d667d95 
  src/MainWindow.cpp 07dca94 
  src/core-impl/playlists/providers/user/UserPlaylistProvider.cpp e19769d 
  src/core-impl/playlists/types/file/PlaylistFile.cpp c327082 
  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 8f4669a 
  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 109758: Asx playlist implementation.

2013-04-15 Thread Commit Hook

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


This review has been submitted with commit 
e1eddb4a00b0d5bece5b9eb59e95b460c762 by Matěj Laitl on behalf of Tatjana 
Gornak to branch master.

- Commit Hook


On April 14, 2013, 5:40 p.m., Tatjana Gornak wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109758/
> ---
> 
> (Updated April 14, 2013, 5:40 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 ef6790f 
>   src/CMakeLists.txt d667d95 
>   src/MainWindow.cpp 07dca94 
>   src/core-impl/playlists/providers/user/UserPlaylistProvider.cpp e19769d 
>   src/core-impl/playlists/types/file/PlaylistFile.cpp c327082 
>   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 8f4669a 
>   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


Jenkins build is back to stable : amarok_master #343

2013-04-15 Thread KDE CI System
See 

___
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-15 Thread Matěj Laitl

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

Ship it!


Good to go, but it seems that no phonon equalizer effect now supports pre-amp, 
so the second case is hard to fix.

Harsh, also please update ~/.gitconfig to mention your real name and surname, 
not a nickname.

- Matěj Laitl


On April 15, 2013, 8:09 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated April 15, 2013, 8:09 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
> -
> 
>   ChangeLog a278be3 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 28fb256 
>   src/dialogs/EqualizerDialog.cpp f42a033 
>   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 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-15 Thread Commit Hook

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

(Updated April 15, 2013, 8:09 p.m.)


Status
--

This change has been marked as submitted.


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
-

  ChangeLog a278be3 
  src/EngineController.h 5de4beb 
  src/EngineController.cpp 28fb256 
  src/dialogs/EqualizerDialog.cpp f42a033 
  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 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-15 Thread Commit Hook

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


This review has been submitted with commit 
e344f608faf236e8faa455e0516f55d4ffeee80c by Matěj Laitl on behalf of Harsh 
Gupta to branch master.

- Commit Hook


On April 15, 2013, 3:55 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated April 15, 2013, 3:55 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
> -
> 
>   ChangeLog a278be3 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 28fb256 
>   src/dialogs/EqualizerDialog.cpp f42a033 
>   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


Jenkins build became unstable: amarok_master #342

2013-04-15 Thread KDE CI System
See 

___
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-15 Thread Matěj Laitl


> On April 14, 2013, 1:38 p.m., Matěj Laitl wrote:
> > src/dialogs/EqualizerDialog.cpp, lines 100-102
> > 
> >
> > code style: no space between if and (
> 
> Harsh Gupta wrote:
> ooppss !!!
> 
> Matěj Laitl wrote:
> I'll fix it while committin, no need to update the patch.
> 
> Harsh Gupta wrote:
> Actually I have already done that in the next patch

Oh I see, sorry, I was confused.


- Matěj


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


On April 15, 2013, 3:55 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated April 15, 2013, 3:55 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
> -
> 
>   ChangeLog a278be3 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 28fb256 
>   src/dialogs/EqualizerDialog.cpp f42a033 
>   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 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-15 Thread Harsh Gupta


> On April 14, 2013, 7:08 p.m., Matěj Laitl wrote:
> > src/dialogs/EqualizerDialog.cpp, lines 100-102
> > 
> >
> > code style: no space between if and (
> 
> Harsh Gupta wrote:
> ooppss !!!
> 
> Matěj Laitl wrote:
> I'll fix it while committin, no need to update the patch.

Actually I have already done that in the next patch


- Harsh


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


On April 15, 2013, 9:25 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated April 15, 2013, 9:25 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
> -
> 
>   ChangeLog a278be3 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 28fb256 
>   src/dialogs/EqualizerDialog.cpp f42a033 
>   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 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-15 Thread Matěj Laitl


> On April 14, 2013, 1:38 p.m., Matěj Laitl wrote:
> > src/dialogs/EqualizerDialog.cpp, lines 100-102
> > 
> >
> > code style: no space between if and (
> 
> Harsh Gupta wrote:
> ooppss !!!

I'll fix it while committin, no need to update the patch.


- Matěj


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


On April 15, 2013, 3:55 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated April 15, 2013, 3:55 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
> -
> 
>   ChangeLog a278be3 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 28fb256 
>   src/dialogs/EqualizerDialog.cpp f42a033 
>   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 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-15 Thread Harsh Gupta


> On March 29, 2013, 12:03 a.m., Harsh Gupta wrote:
> > src/EngineController.cpp, line 793
> > 
> >
> > How am I suppose to stage changes in a line ( line 791 ) in which a 
> > variable is first renamed and then get deleted ? Should I commit all the 
> > changes again in order to make two different patches ?
> 
> Harsh Gupta wrote:
> Oops... sorry for poor tagging !!! (first time  :P)
> Line no. is actually 791.
> 
> Matěj Laitl wrote:
> `git add -p` allows you to edit the hunks while staging, so you can use 
> it to "generate" changes that "annihilated". From "oldVarName" -> "" you can 
> create "oldVarname" -> "newVarName" -> "". (where oldVarName would be in 
> tree, newVarName would be in index (staged to commit) and "" would be in 
> working tree)
> 
> But perhaps this is too complicated for an user new to git. You may 
> instead do:
> 1. start a new branch starting just a commit before your changes: git 
> branch newbranch oldbranch^ (or master^ if you've worked directly in master)
> 2. perform the exact same rename using your IDE (should be quick to do), 
> commit
> 3. check out the working tree of oldbranch (master?) without switching to 
> it: git checkout oldbranch -- .
> 4. `git diff` should show the "real" changes w/thout the renames; commit
> 5. now you have 2 commits, yay! Ensure that both commits build.
> 
> Harsh Gupta wrote:
> Should I submit both the patches in this review request one by one ?
> PS: Please ignore my latest patch. I was expecting two different patches 
> (one as a parent diff) but review board merged the two patches.
> 
> Matěj Laitl wrote:
> > Should I submit both the patches in this review request one by one?
> 
> Please submit the renaming patch as new review request here. Indicate in 
> this review request that it depends on the renaming one. (and optionally 
> update it so that it applied on top of the renaming patch - but this seems 
> already done)
> 
> > I was just wondering what should I write in the Changelog file?
> 
> BUGFIXES:
>  * Pre-aplifier in equalizer is now only enabled if it is actually 
> supported; patch by Harsh Gupta. (BR 301311)
> 
> Harsh Gupta wrote:
> I have created a new review request for the renaming patch. Should I wait 
> for that patch to get shipped ? 
> 
> PS : Sorry for the late response. University projects kept me occupied :(
> 
> Matěj Laitl wrote:
> > I have created a new review request for the renaming patch. Should I 
> wait for that patch to get shipped?
> 
> It already is. :-) 
> 
> > PS: Sorry for the late response. University projects kept me occupied :(
> 
> No problem, glad to know you weren't hit by a bus. Actual review below.

:D


- Harsh


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


On April 15, 2013, 9:25 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated April 15, 2013, 9:25 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
> -
> 
>   ChangeLog a278be3 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 28fb256 
>   src/dialogs/EqualizerDialog.cpp f42a033 
>   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 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-15 Thread Harsh Gupta


> On April 14, 2013, 7:08 p.m., Matěj Laitl wrote:
> > src/dialogs/EqualizerDialog.cpp, lines 100-102
> > 
> >
> > code style: no space between if and (

ooppss !!!


- Harsh


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


On April 15, 2013, 9:25 p.m., Harsh Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108995/
> ---
> 
> (Updated April 15, 2013, 9:25 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
> -
> 
>   ChangeLog a278be3 
>   src/EngineController.h 5de4beb 
>   src/EngineController.cpp 28fb256 
>   src/dialogs/EqualizerDialog.cpp f42a033 
>   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 108995: FIX Pre-amplifier in equalizer doesn't work

2013-04-15 Thread Harsh Gupta

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

(Updated April 15, 2013, 9:25 p.m.)


Review request for Amarok.


Changes
---

All issues resolved.


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 (updated)
-

  ChangeLog a278be3 
  src/EngineController.h 5de4beb 
  src/EngineController.cpp 28fb256 
  src/dialogs/EqualizerDialog.cpp f42a033 
  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 109817: JJ 313649 - No warning if there are no permissions to read file

2013-04-15 Thread Matěj Laitl


> On April 14, 2013, 12:30 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/daap/DaapMeta.cpp, lines 77-85
> > 
> >
> > Please remove this now redundant method - the base implementation + 
> > custom notPlayableReason() will suffice now.
> 
> Anmol Ahuja wrote:
> But wouldn't that lead to a lot of unnecessary overhead, when there are 
> several unplayable tracks? Each call to isPlayable() will generate a 
> non-empty QString, malloc calls and all.
> 
> PS: I'll get to work on this and the other patch [ and hopefully get down 
> to discussing my GSoC proposal :) ] on Wednesday, exams again.

> But wouldn't that lead to a lot of unnecessary overhead, when there are 
> several unplayable tracks? Each call to isPlayable() will generate a 
> non-empty QString, malloc calls and all.

A bit of overhead: yes, but I think we can assume QString creation and i18n() 
calls are already well-optimized. However, this is only a tiny bit of time 
spent in the function - the Solid::Networking::status() call (which may result 
in DBus method call) is already orders of magnitude slower than QString 
handling. Same applied for methods that check the filesystem for 
existence/type/permissions..

> PS: I'll get to work on this and the other patch [ and hopefully get down to 
> discussing my GSoC proposal :) ] on Wednesday, exams again.

No problem, amount of patches you've submitted is already amazing. Good luck 
with your exams.


- Matěj


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


On April 13, 2013, 1:38 p.m., Anmol Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109817/
> ---
> 
> (Updated April 13, 2013, 1:38 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/daap/DaapMeta.cpp e66afb7 
>   src/core-impl/collections/daap/DaapMeta.h 9a9c257 
>   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/meta/Meta.h f86cf39 
>   src/playlist/PlaylistModel.cpp 246b9a1 
>   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-15 Thread Anmol Ahuja


> On April 14, 2013, 6 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/daap/DaapMeta.cpp, lines 77-85
> > 
> >
> > Please remove this now redundant method - the base implementation + 
> > custom notPlayableReason() will suffice now.

But wouldn't that lead to a lot of unnecessary overhead, when there are several 
unplayable tracks? Each call to isPlayable() will generate a non-empty QString, 
malloc calls and all.

PS: I'll get to work on this and the other patch [ and hopefully get down to 
discussing my GSoC proposal :) ] on Wednesday, exams again.


- Anmol


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


On April 13, 2013, 7:08 p.m., Anmol Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109817/
> ---
> 
> (Updated April 13, 2013, 7:08 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/daap/DaapMeta.cpp e66afb7 
>   src/core-impl/collections/daap/DaapMeta.h 9a9c257 
>   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/meta/Meta.h f86cf39 
>   src/playlist/PlaylistModel.cpp 246b9a1 
>   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 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Alberto Villa

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



src/musicbrainz/MusicBrainzTags.cpp


Is this too hacky?



src/musicbrainz/MusicBrainzTags.cpp


I could tolerate the view knowing the specific model, but know it also 
needs to know if it's behind a proxy. I really don't like this approach, I'd 
rather prefer it to refernce only QAbstractItemModel like in other methods. I 
could move isChcked() and checkAllFromRelease() to the view (making them a bit 
more complex), but I'm not sure they belong there.

Any advice?


- Alberto Villa


On April 15, 2013, 12:51 p.m., Alberto Villa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105290/
> ---
> 
> (Updated April 15, 2013, 12:51 p.m.)
> 
> 
> Review request for Amarok and Sergey Ivanov.
> 
> 
> Description
> ---
> 
> The attached patch addresses several issues and brings some improvements, 
> listed below.
> 
> Web service 2 is now being used in place of the deprecated version 1: disc 
> number and artist credit are now better defined. This alone required a 
> complete rewrite of MusicBrainzXmlParser.(cpp|h).
> 
> Make track search query more complicated and let it handle some mistakes 
> (documented in code).
> 
> As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton 
> Howard) are returned splitted (i.e., they do not have one page, but each one 
> has its own), the "Go to Artist Page" menu button was changed to a 
> KActionMenu to support showing a sub list.
> 
> Get track title from release information, as different releases can change 
> track title (e.g., adding (single edit) or similar).
> 
> Instead of doing *release* requests to get album artist and year, do *release 
> group* requests. It often means fewer requests will be done (and each one 
> takes at least 1 second, so...).
> See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
> http://tickets.musicbrainz.org/browse/SEARCH-218
> 
> Reimplement Levenshtein distance algorithm using a more efficient version 
> (pirated from Wikibooks as well as the current one).
> 
> Tracks without results are now listed and disabled, to let user know what 
> results are missing. Prior to this, you had to count them by hand.
> 
> File name is shown in track tooltip. Helpful when existing tags are equivocal.
> 
> Selected results are shown in bold font to be easily spotted.
> 
> Track number, track count, disc number (when existing) and release year 
> (first release date from the release group, actually) are now shown to help 
> distinguish among results. This means that all the fetched information is now 
> shown to the user.
> 
> Tracks are sorted by file name (fair choice). It's done after each insertion, 
> but since insertions are quite sparse, it doesn't cause any performance 
> problem.
> 
> Toolbar features "Select Best Matches" and "Deselect", the latter being new, 
> and the former being moved here from the hidden header button. To keep it not 
> too wide, Expand and Collapse buttons were modified to be KActionMenu. They 
> now require two clicks, but are easier to distinguish (I always had to stop 
> and read them to distinguish between "Collapse All" and "Collapse Chosen", 
> for example), so the time needed to click them looks mostly the same as 
> before.
> 
> The progress bar now gets to 100% even if MusicDNS search is disabled.
> 
> All releases per track are processed instead of only the best one. I'll 
> explain better. Each track is associated to several releases. In current 
> code, all the releases are parsed, but only the one with the highest score is 
> returned to the dialog. This excludes lots of good results. Now, all of them 
> can be returned. This obviously can increase the number of returned resuts, 
> but that's acceptable given that, before, you were not able to tag a lot of 
> tracks. Also, two following paragraphs are about two features that mitigate 
> this "problem".
> 
> Tracks with the same visible information are merged. There's no point in 
> showing several tracks with same title, artist, album name and artist, album 
> year, disc number, track count and track number, as the user will not be able 
> to distinguish among them. Just merge them into only one result, keeping a 
> list of artist, release and track IDs (they are and will be used). Also, the 
> score is updated to reflect the higher one. The "Go to ... Page" menu buttons 
> currently link to the top result (i.e., highest scoring) IDs, but in the 
> future I might add support for showing a list.
> 
> A "Select B

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Alberto Villa


> On Sept. 30, 2012, 1:58 p.m., Matěj Laitl wrote:
> > Screenshot: 
> > 
> >
> > I think you can use KIcon( "document-revert" ) for this button.

Just wondering: the icon suggests reverting to latest "saved/known" state; 
wouldn't a "cleanup"-like icon be better?


> On Sept. 30, 2012, 1:58 p.m., Matěj Laitl wrote:
> > Screenshot: 
> > 
> >
> > I think you can use "toold-wizard" icon here.
> 
> Matěj Laitl wrote:
> *tools-wizard

Now the label is hidden; what should I do? Force it visible?

Also, if you agree, I'd like to reference the "Choose best results from this 
album" action in the tooltip here.


- Alberto


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


On April 15, 2013, 12:51 p.m., Alberto Villa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105290/
> ---
> 
> (Updated April 15, 2013, 12:51 p.m.)
> 
> 
> Review request for Amarok and Sergey Ivanov.
> 
> 
> Description
> ---
> 
> The attached patch addresses several issues and brings some improvements, 
> listed below.
> 
> Web service 2 is now being used in place of the deprecated version 1: disc 
> number and artist credit are now better defined. This alone required a 
> complete rewrite of MusicBrainzXmlParser.(cpp|h).
> 
> Make track search query more complicated and let it handle some mistakes 
> (documented in code).
> 
> As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton 
> Howard) are returned splitted (i.e., they do not have one page, but each one 
> has its own), the "Go to Artist Page" menu button was changed to a 
> KActionMenu to support showing a sub list.
> 
> Get track title from release information, as different releases can change 
> track title (e.g., adding (single edit) or similar).
> 
> Instead of doing *release* requests to get album artist and year, do *release 
> group* requests. It often means fewer requests will be done (and each one 
> takes at least 1 second, so...).
> See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
> http://tickets.musicbrainz.org/browse/SEARCH-218
> 
> Reimplement Levenshtein distance algorithm using a more efficient version 
> (pirated from Wikibooks as well as the current one).
> 
> Tracks without results are now listed and disabled, to let user know what 
> results are missing. Prior to this, you had to count them by hand.
> 
> File name is shown in track tooltip. Helpful when existing tags are equivocal.
> 
> Selected results are shown in bold font to be easily spotted.
> 
> Track number, track count, disc number (when existing) and release year 
> (first release date from the release group, actually) are now shown to help 
> distinguish among results. This means that all the fetched information is now 
> shown to the user.
> 
> Tracks are sorted by file name (fair choice). It's done after each insertion, 
> but since insertions are quite sparse, it doesn't cause any performance 
> problem.
> 
> Toolbar features "Select Best Matches" and "Deselect", the latter being new, 
> and the former being moved here from the hidden header button. To keep it not 
> too wide, Expand and Collapse buttons were modified to be KActionMenu. They 
> now require two clicks, but are easier to distinguish (I always had to stop 
> and read them to distinguish between "Collapse All" and "Collapse Chosen", 
> for example), so the time needed to click them looks mostly the same as 
> before.
> 
> The progress bar now gets to 100% even if MusicDNS search is disabled.
> 
> All releases per track are processed instead of only the best one. I'll 
> explain better. Each track is associated to several releases. In current 
> code, all the releases are parsed, but only the one with the highest score is 
> returned to the dialog. This excludes lots of good results. Now, all of them 
> can be returned. This obviously can increase the number of returned resuts, 
> but that's acceptable given that, before, you were not able to tag a lot of 
> tracks. Also, two following paragraphs are about two features that mitigate 
> this "problem".
> 
> Tracks with the same visible information are merged. There's no point in 
> showing several tracks with same title, artist, album name and artist, album 
> year, disc number, track count and track number, as the user will not be able 
> to distinguish among them. Just merge them into only one result, keeping a 
> list of artist, release and track IDs (they are and will be used). Also, the 
> score is updated to reflect the higher one. The "Go to ... Page" menu buttons 
> currently lin

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Alberto Villa

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

(Updated April 15, 2013, 12:51 p.m.)


Review request for Amarok and Sergey Ivanov.


Changes
---

New patch attached to ask for some help, it's uncomplete and could be 
unbuildable. Specific questions are posted below.


Description
---

The attached patch addresses several issues and brings some improvements, 
listed below.

Web service 2 is now being used in place of the deprecated version 1: disc 
number and artist credit are now better defined. This alone required a complete 
rewrite of MusicBrainzXmlParser.(cpp|h).

Make track search query more complicated and let it handle some mistakes 
(documented in code).

As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton Howard) 
are returned splitted (i.e., they do not have one page, but each one has its 
own), the "Go to Artist Page" menu button was changed to a KActionMenu to 
support showing a sub list.

Get track title from release information, as different releases can change 
track title (e.g., adding (single edit) or similar).

Instead of doing *release* requests to get album artist and year, do *release 
group* requests. It often means fewer requests will be done (and each one takes 
at least 1 second, so...).
See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
http://tickets.musicbrainz.org/browse/SEARCH-218

Reimplement Levenshtein distance algorithm using a more efficient version 
(pirated from Wikibooks as well as the current one).

Tracks without results are now listed and disabled, to let user know what 
results are missing. Prior to this, you had to count them by hand.

File name is shown in track tooltip. Helpful when existing tags are equivocal.

Selected results are shown in bold font to be easily spotted.

Track number, track count, disc number (when existing) and release year (first 
release date from the release group, actually) are now shown to help 
distinguish among results. This means that all the fetched information is now 
shown to the user.

Tracks are sorted by file name (fair choice). It's done after each insertion, 
but since insertions are quite sparse, it doesn't cause any performance problem.

Toolbar features "Select Best Matches" and "Deselect", the latter being new, 
and the former being moved here from the hidden header button. To keep it not 
too wide, Expand and Collapse buttons were modified to be KActionMenu. They now 
require two clicks, but are easier to distinguish (I always had to stop and 
read them to distinguish between "Collapse All" and "Collapse Chosen", for 
example), so the time needed to click them looks mostly the same as before.

The progress bar now gets to 100% even if MusicDNS search is disabled.

All releases per track are processed instead of only the best one. I'll explain 
better. Each track is associated to several releases. In current code, all the 
releases are parsed, but only the one with the highest score is returned to the 
dialog. This excludes lots of good results. Now, all of them can be returned. 
This obviously can increase the number of returned resuts, but that's 
acceptable given that, before, you were not able to tag a lot of tracks. Also, 
two following paragraphs are about two features that mitigate this "problem".

Tracks with the same visible information are merged. There's no point in 
showing several tracks with same title, artist, album name and artist, album 
year, disc number, track count and track number, as the user will not be able 
to distinguish among them. Just merge them into only one result, keeping a list 
of artist, release and track IDs (they are and will be used). Also, the score 
is updated to reflect the higher one. The "Go to ... Page" menu buttons 
currently link to the top result (i.e., highest scoring) IDs, but in the future 
I might add support for showing a list.

A "Select Best Matches from This Album" menu button was added. It matches the 
top result release ID in other *unselected* results, to make album tagging much 
easier. Since a result might reference many release IDs (see the paragraph 
above), the match is done on the whole list of them. This is highly recommended 
over the "Select Best Matches" toolbar button.

Process MusicDNS results just like MusicBrainz ones (i.e., do not duplicate the 
logic). Simply, as they will not carry existing tags, they'll end up being 
checked only by track length.

Rewrite MusicDNS result matching method in the list view. Actually, remove it 
(it was checking for equal track ID, but that is a very weak method, especially 
after my updates), and use the matching system described above.

Make MusicBrainzTagsView implementation agnostic (i.e., never use 
MusicBrainzTagsItem inside it, and make use of UserRole).

Remove stale code from MusicBrainzFinder.cpp and M

Re: Review Request 105290: Make MusicBrainz tagger more accurate and easy to use

2013-04-15 Thread Alberto Villa

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


- Alberto Villa


On Sept. 26, 2012, 12:26 a.m., Alberto Villa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105290/
> ---
> 
> (Updated Sept. 26, 2012, 12:26 a.m.)
> 
> 
> Review request for Amarok and Sergey Ivanov.
> 
> 
> Description
> ---
> 
> The attached patch addresses several issues and brings some improvements, 
> listed below.
> 
> Web service 2 is now being used in place of the deprecated version 1: disc 
> number and artist credit are now better defined. This alone required a 
> complete rewrite of MusicBrainzXmlParser.(cpp|h).
> 
> Make track search query more complicated and let it handle some mistakes 
> (documented in code).
> 
> As in web service 2 multiple artists (e.g., Hans Zimmer & James Newton 
> Howard) are returned splitted (i.e., they do not have one page, but each one 
> has its own), the "Go to Artist Page" menu button was changed to a 
> KActionMenu to support showing a sub list.
> 
> Get track title from release information, as different releases can change 
> track title (e.g., adding (single edit) or similar).
> 
> Instead of doing *release* requests to get album artist and year, do *release 
> group* requests. It often means fewer requests will be done (and each one 
> takes at least 1 second, so...).
> See http://tickets.musicbrainz.org/browse/SEARCH-214 and 
> http://tickets.musicbrainz.org/browse/SEARCH-218
> 
> Reimplement Levenshtein distance algorithm using a more efficient version 
> (pirated from Wikibooks as well as the current one).
> 
> Tracks without results are now listed and disabled, to let user know what 
> results are missing. Prior to this, you had to count them by hand.
> 
> File name is shown in track tooltip. Helpful when existing tags are equivocal.
> 
> Selected results are shown in bold font to be easily spotted.
> 
> Track number, track count, disc number (when existing) and release year 
> (first release date from the release group, actually) are now shown to help 
> distinguish among results. This means that all the fetched information is now 
> shown to the user.
> 
> Tracks are sorted by file name (fair choice). It's done after each insertion, 
> but since insertions are quite sparse, it doesn't cause any performance 
> problem.
> 
> Toolbar features "Select Best Matches" and "Deselect", the latter being new, 
> and the former being moved here from the hidden header button. To keep it not 
> too wide, Expand and Collapse buttons were modified to be KActionMenu. They 
> now require two clicks, but are easier to distinguish (I always had to stop 
> and read them to distinguish between "Collapse All" and "Collapse Chosen", 
> for example), so the time needed to click them looks mostly the same as 
> before.
> 
> The progress bar now gets to 100% even if MusicDNS search is disabled.
> 
> All releases per track are processed instead of only the best one. I'll 
> explain better. Each track is associated to several releases. In current 
> code, all the releases are parsed, but only the one with the highest score is 
> returned to the dialog. This excludes lots of good results. Now, all of them 
> can be returned. This obviously can increase the number of returned resuts, 
> but that's acceptable given that, before, you were not able to tag a lot of 
> tracks. Also, two following paragraphs are about two features that mitigate 
> this "problem".
> 
> Tracks with the same visible information are merged. There's no point in 
> showing several tracks with same title, artist, album name and artist, album 
> year, disc number, track count and track number, as the user will not be able 
> to distinguish among them. Just merge them into only one result, keeping a 
> list of artist, release and track IDs (they are and will be used). Also, the 
> score is updated to reflect the higher one. The "Go to ... Page" menu buttons 
> currently link to the top result (i.e., highest scoring) IDs, but in the 
> future I might add support for showing a list.
> 
> A "Select Best Matches from This Album" menu button was added. It matches the 
> top result release ID in other *unselected* results, to make album tagging 
> much easier. Since a result might reference many release IDs (see the 
> paragraph above), the match is done on the whole list of them. This is highly 
> recommended over the "Select Best Matches" toolbar button.
> 
> Process MusicDNS results just like MusicBrainz ones (i.e., do not duplicate 
> the logic). Simply, as they will not carry existing tags, they'll end up 
> being checked only by track length.
> 
> Rewrite MusicDNS result matching metho

Re: CUE sheets in collection still not working :(

2013-04-15 Thread Abhinandan Ramprasath
I'm still working on the patch for the mp4 chapter support.
The one I submitted the other day was kind of incomplete and realized my
mistakes and it turned out be way bigger than I anticipated. I would love
to take this up as a GSOC project. Since, I got really involved in this one
and because of college, I really couldn't find any time to work on anything
any other bug(i'll try to make some time asap and solve some others).

P.S I'll make a pull request for taglib in the coming days :)




On Mon, Apr 15, 2013 at 12:58 PM, Bart Cerneels wrote:

> On Sun, Apr 14, 2013 at 12:11 PM, Matěj Laitl  wrote:
> >
> > On 11. 4. 2013 Myriam Schweingruber wrote:
> > > 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.
> >
> > Bart, what about turning this into "Unified CUE file and Audiobook
> chapter
> > support in Amarok" GSoC project?
> >
> > I've thought about it and CUE files and audiobooks with chapters are
> > essentially the same, just the medium to store the the chapters/parts is
> > different, but inside Amarok these can share the very same code paths.
>
> Was about to suggest doing it as a GSoC. I can give some suggestions
> but can't mentor unfortunately. If someone steps up to mentor, contact
> me.
>
> P.S. My amarok emails are filtered out of my inbox ATM, please CC me
> if you want to make sure I see it.
> ___
> Amarok-devel mailing list
> Amarok-devel@kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel
>
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel


Re: CUE sheets in collection still not working :(

2013-04-15 Thread Bart Cerneels
On Sun, Apr 14, 2013 at 12:11 PM, Matěj Laitl  wrote:
>
> On 11. 4. 2013 Myriam Schweingruber wrote:
> > 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.
>
> Bart, what about turning this into "Unified CUE file and Audiobook chapter
> support in Amarok" GSoC project?
>
> I've thought about it and CUE files and audiobooks with chapters are
> essentially the same, just the medium to store the the chapters/parts is
> different, but inside Amarok these can share the very same code paths.

Was about to suggest doing it as a GSoC. I can give some suggestions
but can't mentor unfortunately. If someone steps up to mentor, contact
me.

P.S. My amarok emails are filtered out of my inbox ATM, please CC me
if you want to make sure I see it.
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel


Re: CUE sheets in collection still not working :(

2013-04-15 Thread Lukáš Lalinský
I'm not sure how exactly are chapters stored in MP4 files, but I don't
expect it to be a too large change. I don't see a reason why it would
be not possible to implement it as a part of the GSoC project.

Btw, there has been a post on the TagLib mailing list about
implementing the CHAP frame on ID3. I'm not sure if this is triggered
by the Amarok issue, or completely unrelated:

http://article.gmane.org/gmane.comp.kde.devel.taglib/2427

Lukas

On Sun, Apr 14, 2013 at 12:11 PM, Matěj Laitl  wrote:
> On 11. 4. 2013 Myriam Schweingruber wrote:
>> 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.
>
> Bart, what about turning this into "Unified CUE file and Audiobook chapter
> support in Amarok" GSoC project?
>
> I've thought about it and CUE files and audiobooks with chapters are
> essentially the same, just the medium to store the the chapters/parts is
> different, but inside Amarok these can share the very same code paths.
>
> It would perhaps require extending TagLib a bit, so it could form a nice GSoC
> project. What do you think? Would you be able to mentor?
>
> [CCing TagLib devs: can a part of our GSoC project be to implement chapter
> metadata reading for audiobooks? (mostly mp4)]
>
> Matěj
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel