<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/104294/">http://git.reviewboard.kde.org/r/104294/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 16th, 2012, 12:15 p.m., <b>Ralf Engels</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just going through all the review requests again.
With your patch the following auto test is failing: testIdentifyCompilationInMultipleDirectories</pre>
</blockquote>
<p>On August 16th, 2012, 5:36 p.m., <b>Alexey Neyman</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Which is, I guess, expected - since it was the purpose of this patch to detect albums as separate if they reside in separate directories. As far as I can see from the test, it sets "track artist" on each of the tracks, but not "album artists". So, the test should be adjusted to set album artist to the same value for all these tracks (see your example above vs. mine, above).
I'll try to update the test - but I won't be able to run it, I still haven't succeeded in using google mock on Kubuntu.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">When talking about the example:
In my example the Album artist should be set to "Michael Jackson" (also for the audio comment from Quincy Jones).
In your example the Album artists should be set to "Abba" and "Chris Norman".
How about continuing like this: add a new test case that checks if Albums with different album artists are not merged.
Then we change the code so that also this case works.
If you split up albums with different track artist, then you are in principle eliminating all compilations, right?</pre>
<br />
<p>- Ralf</p>
<br />
<p>On March 16th, 2012, 12:13 a.m., Alexey Neyman wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok.</div>
<div>By Alexey Neyman.</div>
<p style="color: grey;"><i>Updated March 16, 2012, 12:13 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Older amarok used the following heuristics to determine whether a particular
album is a compilation (from a comment in
amarok-2.2.1/src/collection/sqlcollection/ScanResultProcessor.cpp):
//using the following heuristics:
//if more than one album is in the dir, use the artist of each track as albumartist
//if all tracks have the same artist, use it as albumartist
//try to find the albumartist A: tracks must have the artist A or A feat. B (and variants)
//if no albumartist could be found, it's a compilation
However, more recent Amarok versions started to merge different albums
with different artist in separate directories together, as explained above.
Amarok started to assume all albums with same name to be compilations
(even if in separate directories) since the following commit:
dfd8b457d7094144563c51b2528afdbe23ffc344
Ralf Engels
Fix all collection scanner auto tests.
Now, amarok first scans all directories (sorting albums by the name)
and then tries to process *album names*, one at a time. If it finds
more than one instance of an album name, it assumes it to be a compilation.
Thus, it lost the heuristics in employed before ("if more than one album
is in the dir...").
While it is still possible to force the right behavior
by selecting "Do not show under Various Artists" for each of the erroneous
albums, it would still be better to restore the original heuristics as there
may be lots of albums merged this way. I think the old heuristics made sense
(why would albums be put into separate directories otherwise, if they are
a single compilation album?).
The attached patch restores the following logic: If any given directory
contains tracks that were sorted into a single album and and that album
was not created as a compilation (i.e. it has non-empty artists), this
album is excluded from being merged with other albums to create a "compilation".</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/core-impl/collections/db/ScanResultProcessor.cpp <span style="color: grey">(4f02a16)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104294/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>