Discussion:
Feature request
Semen
2012-05-14 13:42:04 UTC
Permalink
Hi!
Could you add ALBUMSORT tag support (TSOA frame in ID3)?

And what about CUE sheets support? I don't use CUEs currently, I just tried
to test this feauture but I can't load any CUE sheet in cmus...

I suppose cmus should load CUEs automatically when user adds music to
library (It should support both internal CUEs ("CUESHEET" tag) and external
with option in settings "Prefer internal or external CUE"). I think that it
is hard to implement such ideal behaviour, especially with current library
format.

P.S. Execuse me for my poor English.
Johannes Weißl
2012-05-14 19:25:33 UTC
Permalink
Hi Semen,
Post by Semen
Could you add ALBUMSORT tag support (TSOA frame in ID3)?
Very good suggestion, I was surprised we don't support that already. I
will implement it and post a patch soon!
Post by Semen
And what about CUE sheets support? I don't use CUEs currently, I just tried
to test this feauture but I can't load any CUE sheet in cmus...
I guess Gregory is the best to answer this question, since he
implemented CUE support. As far as I know you don't load CUE files
directly, but they are detected. E.g. you have:
- full_album.flac
- full_album.flac.cue
If you load "full_album.flac", the CUE file is used instead so you have
multiple tracks instead of one file.


Greetings,
Johannes
Gregory Petrosyan
2012-05-14 20:35:32 UTC
Permalink
Post by Johannes Weißl
Post by Semen
And what about CUE sheets support? I don't use CUEs currently, I just tried
to test this feauture but I can't load any CUE sheet in cmus...
I guess Gregory is the best to answer this question, since he
implemented CUE support. As far as I know you don't load CUE files
- full_album.flac
- full_album.flac.cue
If you load "full_album.flac", the CUE file is used instead so you have
multiple tracks instead of one file.
Yeah — when loading «track.flac», cmus checks for «track.cue», and
tries to use it. So all you have to do is to add your flac files (if
cue files are named appropriately), everything should happen
automatically.

Note, that this feature is not released yet, so you have to use cmus
from git («pu» branch, http://cmus.sourceforge.net/#download). Also,
CUE parsing is pretty basic (regarding Unicode etc.) — I used libcue
instead of writing cue parser myself, and libcue is pretty poor
library.

       Gregory
Johannes Weißl
2012-06-14 21:30:38 UTC
Permalink
Post by Gregory Petrosyan
As far as I know you don't load CUE files directly, but they are
- full_album.flac
- full_album.flac.cue
Yeah — when loading «track.flac», cmus checks for «track.cue», and
tries to use it. So all you have to do is to add your flac files (if
cue files are named appropriately), everything should happen
automatically.
You are right, it is not "track.flac.cue" but "track.cue". But I think
to remember that I've asked about inclusion of "track.flac.cue", and you
agreed. So here is my patch :-).

Johannes
Johannes Weißl
2012-06-14 21:31:11 UTC
Permalink
---
cue_utils.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/cue_utils.c b/cue_utils.c
index c846f6b..73aef41 100644
--- a/cue_utils.c
+++ b/cue_utils.c
@@ -54,7 +54,7 @@ Cd *cue_parse_file__no_stderr_garbage(FILE *f)

char *associated_cue(const char *filename)
{
- char *f;
+ FILE *fp;
const char *ext;
char buf[4096] = {0};
const char *dot;
@@ -67,10 +67,13 @@ char *associated_cue(const char *filename)
if (dot == NULL)
return NULL;

- f = xstrndup(filename, dot - filename);
- snprintf(buf, sizeof buf, "%s.cue", f);
+ snprintf(buf, sizeof buf, "%.*s.cue", (int) (dot - filename), filename);
+ fp = fopen(buf, "r");
+ if (!fp)
+ snprintf(buf, sizeof buf, "%s.cue", filename);
+ else
+ fclose(fp);

- free(f);
return xstrdup(buf);
}
--
1.7.10
Gregory Petrosyan
2012-06-15 08:53:53 UTC
Permalink
Post by Johannes Weißl
- snprintf(buf, sizeof buf, "%s.cue", f);
+ snprintf(buf, sizeof buf, "%.*s.cue", (int) (dot - filename), filename);
Wow, didn't know about this trick!

The patch looks a bit hacky (associated_cue() has now an fopen() inside it, but
does not guarantee that the path being returned exists — go figure :-)
but that's just a nitpicking from my side — the patch is short and sweet.

Thanks a lot — merged to pu!

Gregory
Сергей Сарбаш
2012-06-16 07:18:09 UTC
Permalink
---------- Forwarded message ----------
From: Сергей Сарбаш <***@gmail.com>
Date: 2012/6/16
Subject: Re: [PATCH] cue: add "filename.ext.cue" to the checked paths
To: Gregory Petrosyan <***@gmail.com>


Hello.
Post by Gregory Petrosyan
The patch looks a bit hacky (associated_cue() has now an fopen() inside it, but
does not guarantee that the path being returned exists — go figure :-)
but that's just a nitpicking from my side — the patch is short and sweet.
Thanks a lot — merged to pu!
This night I've tried to build last git but [file.ext.cue + file.ext]
doesn't work. Despite that, [file.cue + file.ext] works perfectly.
I've used "cmus-git" PKGBUILD from AUR. Sometime ago even [file.ext +
file.cue] didn't worked. Maybe need I to wait for a while?
TIA.
--
Truly yours,
Sergey Sarbash.
Gregory Petrosyan
2012-06-16 07:40:31 UTC
Permalink
Post by Сергей Сарбаш
Post by Gregory Petrosyan
The patch looks a bit hacky (associated_cue() has now an fopen() inside it, but
does not guarantee that the path being returned exists — go figure :-)
but that's just a nitpicking from my side — the patch is short and sweet.
Thanks a lot — merged to pu!
This night I've tried to build last git but [file.ext.cue + file.ext]
doesn't work. Despite that, [file.cue + file.ext] works perfectly.
I've used "cmus-git" PKGBUILD from AUR. Sometime ago even [file.ext +
file.cue] didn't worked. Maybe need I to wait for a while?
TIA.
My guess is that cmus-git PKGBUILD uses master branch of the git repository,
while the patch was pushed to the pu (proposed updates) branch. You can either
try to build the pu branch manually (following the instructions from the cmus'
website), or wait a bit — soon this patch will be in the master branch, too.

Gregory
Сергей Сарбаш
2012-06-16 10:06:16 UTC
Permalink
Post by Gregory Petrosyan
My guess is that cmus-git PKGBUILD uses master branch of the git repository,
while the patch was pushed to the pu (proposed updates) branch. You can either
try to build the pu branch manually (following the instructions from the cmus'
website), or wait a bit — soon this patch will be in the master branch, too.
Thank you very much for the explanation. That's really so.
The meaning of the acronym "pu" was incomprehensible for me until now. :)
--
Truly yours,
Sergey Sarbash.
Johannes Weißl
2012-06-19 17:07:13 UTC
Permalink
Post by Gregory Petrosyan
Post by Johannes Weißl
- snprintf(buf, sizeof buf, "%s.cue", f);
+ snprintf(buf, sizeof buf, "%.*s.cue", (int) (dot - filename), filename);
Wow, didn't know about this trick!
Yes, it's pretty irrelevant runtime/memory wise here, but nonetheless :-).
Post by Gregory Petrosyan
The patch looks a bit hacky (associated_cue() has now an fopen() inside it, but
does not guarantee that the path being returned exists — go figure :-)
but that's just a nitpicking from my side — the patch is short and sweet.
Yes, I thought that, too, so first I tried to restructure it (so that
associated_cue() returns FILE), but the name is needed later. Then I
searched for a quick way to check for file existence, but it turns out
fopen() is the way to go.

What do you think about a new associated_cues() that fills a linked list
of suggestions? I can't think of a really clever way to make it less
hacky...


Johannes
Gregory Petrosyan
2012-06-20 07:27:23 UTC
Permalink
Post by Johannes Weißl
Post by Gregory Petrosyan
The patch looks a bit hacky (associated_cue() has now an fopen() inside it, but
does not guarantee that the path being returned exists — go figure :-)
but that's just a nitpicking from my side — the patch is short and sweet.
Yes, I thought that, too, so first I tried to restructure it (so that
associated_cue() returns FILE), but the name is needed later. Then I
searched for a quick way to check for file existence, but it turns out
fopen() is the way to go.
What do you think about a new associated_cues() that fills a linked list
of suggestions? I can't think of a really clever way to make it less
hacky...
In a higher-level language, returning a list will be the way to go,
sure — but in C, I don't think the pain is worth the gain; C naturally
leads to «hacky» code a bit :-)

       Gregory

Johannes Weißl
2012-05-28 12:26:15 UTC
Permalink
Is used to sort albums in tree view (if date is equal/missing).

Tag mapping:
ID3: TSOA/XSOA/TSA
WMA: WM/AlbumSortOrder
MP4: soal

Suggested-by: Semen <***@gmail.com>
---
comment.c | 2 ++
id3.c | 6 ++++++
id3.h | 1 +
lib.h | 2 ++
mp4.c | 2 ++
track_info.c | 1 +
track_info.h | 1 +
tree.c | 25 ++++++++++++++++++++-----
8 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/comment.c b/comment.c
index 12c7ce4..011d3e5 100644
--- a/comment.c
+++ b/comment.c
@@ -160,6 +160,7 @@ int comments_get_date(const struct keyval *comments, const char *key)
static const char *interesting[] = {
"artist", "album", "title", "tracknumber", "discnumber", "genre",
"date", "compilation", "albumartist", "artistsort", "albumartistsort",
+ "albumsort",
"originaldate",
"replaygain_track_gain",
"replaygain_track_peak",
@@ -184,6 +185,7 @@ static struct {
{ "WM/Year", "date" },
{ "WM/ArtistSortOrder", "artistsort" },
{ "WM/AlbumArtistSortOrder", "albumartistsort" },
+ { "WM/AlbumSortOrder", "albumsort" },
{ "WM/OriginalReleaseYear", "originaldate" },
{ "WM/Media", "media" },
{ "sourcemedia", "media" },
diff --git a/id3.c b/id3.c
index 500a716..30cc4e7 100644
--- a/id3.c
+++ b/id3.c
@@ -248,6 +248,7 @@ const char * const id3_key_names[NUM_ID3_KEYS] = {
"albumartist",
"artistsort",
"albumartistsort",
+ "albumsort",
"compilation",
"replaygain_track_gain",
"replaygain_track_peak",
@@ -547,6 +548,7 @@ static struct {
{ "TDRL", ID3_DATE }, // release date
{ "TDOR", ID3_ORIGINALDATE }, // original release date
{ "TSOP", ID3_ARTISTSORT },
+ { "TSOA", ID3_ALBUMSORT },

/* >= 2.3.0 */
{ "TPE1", ID3_ARTIST },
@@ -559,6 +561,7 @@ static struct {
{ "TPE2", ID3_ALBUMARTIST },
{ "TSO2", ID3_ALBUMARTISTSORT },
{ "XSOP", ID3_ARTISTSORT }, // obsolete
+ { "XSOA", ID3_ALBUMSORT }, // obsolete
{ "TCMP", ID3_COMPILATION },
{ "TORY", ID3_ORIGINALDATE },
{ "TCOM", ID3_COMPOSER },
@@ -580,6 +583,7 @@ static struct {
{ "TRK", ID3_TRACK },
{ "TSP", ID3_ARTISTSORT },
{ "TS2", ID3_ALBUMARTISTSORT },
+ { "TSA", ID3_ALBUMSORT },
{ "TCP", ID3_COMPILATION },

{ "", -1 }
@@ -783,6 +787,8 @@ static void decode_txxx(struct id3tag *id3, const char *buf, int len, int encodi
key = ID3_ALBUMARTIST;
else if (!strcasecmp(out, "albumartistsort"))
key = ID3_ALBUMARTISTSORT;
+ else if (!strcasecmp(out, "albumsort"))
+ key = ID3_ALBUMSORT;
else if (!strcasecmp(out, "compilation"))
key = ID3_COMPILATION;

diff --git a/id3.h b/id3.h
index 89a4454..b786f2b 100644
--- a/id3.h
+++ b/id3.h
@@ -35,6 +35,7 @@ enum id3_key {
ID3_ALBUMARTIST,
ID3_ARTISTSORT,
ID3_ALBUMARTISTSORT,
+ ID3_ALBUMSORT,
ID3_COMPILATION,
ID3_RG_TRACK_GAIN,
ID3_RG_TRACK_PEAK,
diff --git a/lib.h b/lib.h
index ae3dcd0..2cef1cc 100644
--- a/lib.h
+++ b/lib.h
@@ -54,7 +54,9 @@ struct album {

struct artist *artist;
char *name;
+ char *sort_name;
char *collkey_name;
+ char *collkey_sort_name;
/* date of the first track added to this album */
int date;
};
diff --git a/mp4.c b/mp4.c
index e8535aa..4695eb0 100644
--- a/mp4.c
+++ b/mp4.c
@@ -454,6 +454,8 @@ static int mp4_read_comments(struct input_plugin_data *ip_data,
comments_add_const(&c, "artistsort", tags->sortArtist);
if (tags->sortAlbumArtist)
comments_add_const(&c, "albumartistsort", tags->sortAlbumArtist);
+ if (tags->sortAlbum)
+ comments_add_const(&c, "albumsort", tags->sortAlbum);
if (tags->album)
comments_add_const(&c, "album", tags->album);
if (tags->name)
diff --git a/track_info.c b/track_info.c
index 8779bc0..6929898 100644
--- a/track_info.c
+++ b/track_info.c
@@ -69,6 +69,7 @@ void track_info_set_comments(struct track_info *ti, struct keyval *comments) {
ti->comment = keyvals_get_val(comments, "comment");
ti->albumartist = comments_get_albumartist(comments);
ti->artistsort = comments_get_artistsort(comments);
+ ti->albumsort = keyvals_get_val(comments, "albumsort");
ti->is_va_compilation = track_is_va_compilation(comments);
ti->media = keyvals_get_val(comments, "media");

diff --git a/track_info.h b/track_info.h
index 18ddb8c..0e0a5b9 100644
--- a/track_info.h
+++ b/track_info.h
@@ -51,6 +51,7 @@ struct track_info {
const char *comment;
const char *albumartist;
const char *artistsort;
+ const char *albumsort;
const char *media;

char *collkey_artist;
diff --git a/tree.c b/tree.c
index ebbe341..43ec0cf 100644
--- a/tree.c
+++ b/tree.c
@@ -398,12 +398,15 @@ static void artist_free(struct artist *artist)
free(artist);
}

-static struct album *album_new(struct artist *artist, const char *name, int date)
+static struct album *album_new(struct artist *artist, const char *name,
+ const char *sort_name, int date)
{
struct album *album = xnew(struct album, 1);

album->name = xstrdup(name);
+ album->sort_name = sort_name ? xstrdup(sort_name) : NULL;
album->collkey_name = u_strcasecoll_key(name);
+ album->collkey_sort_name = u_strcasecoll_key0(sort_name);
album->date = date;
rb_root_init(&album->track_root);
album->artist = artist;
@@ -414,7 +417,9 @@ static struct album *album_new(struct artist *artist, const char *name, int date
static void album_free(struct album *album)
{
free(album->name);
+ free(album->sort_name);
free(album->collkey_name);
+ free(album->collkey_sort_name);
free(album);
}

@@ -491,9 +496,17 @@ static int special_name_cmp(const char *a, const char *collkey_a,
return strcmp(collkey_a, collkey_b);
}

+static inline const char *album_sort_collkey(const struct album *a)
+{
+ if (a->sort_name)
+ return a->collkey_sort_name;
+
+ return a->collkey_name;
+}
+
static int special_album_cmp(const struct album *a, const struct album *b)
{
- return special_name_cmp(a->name, a->collkey_name, b->name, b->collkey_name);
+ return special_name_cmp(a->name, album_sort_collkey(a), b->name, album_sort_collkey(b));
}

static int special_album_cmp_date(const struct album *a, const struct album *b)
@@ -507,7 +520,7 @@ static int special_album_cmp_date(const struct album *a, const struct album *b)
if (cmp)
return cmp;

- return strcmp(a->collkey_name, b->collkey_name);
+ return strcmp(album_sort_collkey(a), album_sort_collkey(b));
}

/* has to follow the same logic as artist_sort_name() */
@@ -749,6 +762,7 @@ void tree_add_track(struct tree_track *track)
{
const struct track_info *ti = tree_track_info(track);
const char *album_name, *artist_name, *artistsort_name = NULL;
+ const char *albumsort_name = NULL;
struct artist *artist, *new_artist;
struct album *album, *new_album;
int date;
@@ -765,6 +779,7 @@ void tree_add_track(struct tree_track *track)
album_name = tree_album_name(ti);
artist_name = tree_artist_name(ti);
artistsort_name = ti->artistsort;
+ albumsort_name = ti->albumsort;

is_va_compilation = ti->is_va_compilation;
}
@@ -775,12 +790,12 @@ void tree_add_track(struct tree_track *track)
artist = find_artist(new_artist);
if (artist) {
artist_free(new_artist);
- new_album = album_new(artist, album_name, date);
+ new_album = album_new(artist, album_name, albumsort_name, date);
album = find_album(new_album);
if (album)
album_free(new_album);
} else
- new_album = album_new(new_artist, album_name, date);
+ new_album = album_new(new_artist, album_name, albumsort_name, date);

if (artist) {
int changed = 0;
--
1.7.10
Gregory Petrosyan
2012-05-29 19:09:22 UTC
Permalink
Post by Johannes Weißl
Is used to sort albums in tree view (if date is equal/missing).
ID3: TSOA/XSOA/TSA
WMA: WM/AlbumSortOrder
MP4: soal
Thanks a lot, merged to the master (also, I've reverted my FFMPEG
auto-configure disabling patch).

Sorry for the delay!

Gregory
Loading...