Re: [Musicpd-dev-team] Flac embedded cuesheet support
Hi Max, Max Kellermann wrote: Do you know of a better way/have a better idea for enumerating the subtracks instead of track_xxx.flac like i did atm? No, although I'd probably just call it 1, 2, ... without any prefix/suffix. I figured with the prefix the files get sorted in a correct order.. but this is cosmetical and should be configurable as soon as we have cue sheet support. For now, we could say: those virtual sub-songs can only be one level above the main song (which is enough for supporting CUE). So if the file cannot be found, try its parent. If that's a file, handled by a decoder plugin which can handle sub-songs, call method sub_decode() (instead of file_decode()?). Just an idea. So we would have two new api extensions: sub_decode() and scan_directory()? That's ok for me and i'll implement it that way. This should be no problem with the scan_directory api extension. Though i now have to include tag.h, song.h, songvec.h and dirvec.h to flac_plugin.c. Is this ok? Doesn't sound perfect yet, but good enough for now. We could make the scan_directory() method point to the flac_cue_track() method and extend the the tag_dup method to tag the (virtual) files accordingly. That way the code would be separated i think and the includes shouldn't be necessary anymore. 3) Write an parser on your own. Doesn't look too hard, does it? If we write a good library, others might adopt it in the future... that'll be the first CUE library with a decent API then. Hm.. i think it would be a bit hard for me but i got used to C the last weeks so it could be done i guess (also it'd take some time..). The more general problem is that i do not really have a clue about writing a parser. I figured to make something like a fixed size hash table according to the entrys (like PERFORMER, TRACK, etc.) and then have a linked list with entrys attached to the table (similar to separate chaining). But maybe there's a whole different general approach to parsing i am not aware of. Maybe we can fork the cuetools library? There are some good parts which could be used but the parser itself is written in yacc/bison which i don't speak. :) Regards, Jochen signature.asc Description: OpenPGP digital signature -- Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Flac embedded cuesheet support
Hello Max, Max Kellermann wrote: It's a hack, but I think if we cut off some rough edges, we can merge it anyway. Let's get the feature in if it doesn't break anything, and think about migrating to a clean solution after that. Currently i'm working on the scan_directory() method you proposed in one of your first emails. This isn't to hard to do now and we'll get rid of several include dependencies. Do you know of a better way/have a better idea for enumerating the subtracks instead of track_xxx.flac like i did atm? diff --git a/src/input_file.c b/src/input_file.c index b29a412..cd3f2e7 100644 --- a/src/input_file.c +++ b/src/input_file.c @@ -25,6 +25,9 @@ #include string.h #include glib.h +#include ls.h +#include decoder/_flac_common.h + MPD core sources should not include decoder specific headers. But now I understand what you meant with input_flac on IRC - it'll eliminate this dependency. Would it be better if i did something like this: stat() the pathname on NULL: go one level below stat() again until we're either root or have a file. (haven't tried it yet, maybe i'm wrong on this but after a quick look in input_file.c i think this might work as well) diff --git a/src/update.c b/src/update.c index 17059ff..9599e9a 100644 --- a/src/update.c +++ b/src/update.c @@ -35,6 +35,9 @@ #include stats.h #include main.h #include config.h +#include tag.h + +#include decoder/_flac_common.h Same here, although that will be more difficult to get rid of. Let's say you add #ifdef HAVE_FLAC around that code, and we merge it - cleanup (decoder API extension) later. This should be no problem with the scan_directory api extension. Though i now have to include tag.h, song.h, songvec.h and dirvec.h to flac_plugin.c. Is this ok? The general approach of scan_directory could be used for the archive plugin as well i think.. We had a short talk about the cue sheet support on irc. This is going to be a pita imho. :) 1) cuetools are from 2006 and take only a fd as input A possible solution would be to write out embedded cue sheets (like in flacs) to a temporary file with mkstemp() (libc) and use that for i/o. 2) cdrecord has a cue sheet parser. This also takes an fd and it's not a library so using it could become quite difficult. Whatsmore it another ~1250 lines monster i cannot rewrite easily.. This gets me to 3) Write an parser on your own. Here you can find the complete spec: http://digitalx.org/cuesheetsyntax.php As you can see (and also from cuetools/cdrecord) this ain't an easy thing to do. 4) Use a parser lib written in e.g. in C#: http://wyday.com/cuesharp/ Do you really want that? :) As a conclusion for the cue parser/plugin: Writing a parser ourselves would be the best option but also the hardest. I tend to like the cuetools approach best because the specs didn't change and cuetools seem to be usable in an easy way. What i don't like though is having a tmp file.. Looking forward to your reply and greets, Jochen signature.asc Description: OpenPGP digital signature -- Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Flac embedded cuesheet support
Hello Max,
as promised i send the patch against the latest git sources (cloned on
Mo 23 Feb 2009 10:57:36 CET).
This will add support for embedded cue sheets in flac files.
Please send me any comments for improvement, critisim or bugs.
I have commented some parts where i think more work has to be done.
Whatsmore i'm not so happy with it overall, to me it seems more like a
hack rather then something clean.. anyway for the moment it's the best i
can do.
Hope you like it anyway. ;)
Any hints where i could get started to get the subsongs proper tagged?
Max Kellermann wrote:
If you're developing only for the new libflac API
(FLAC_API_VERSION_CURRENT 7), then you don't need any macros, but be
sure your code won't be compiled on older libflac APIs. There are
still people around using those ancient versions..
For the moment my patch isn't #defined for FLAC 7 but i'll change that
asap.
One last question:
Is there a way to access song-tag from the decoder plugin (flac_plugin.c)?
Trying access decoder-stream_tag or decoder-decoder_tag doesn't
work/wouldn't even compile.
Thanks and regards,
Jochen
diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c
index 63dc6e1..7dd0b8a 100644
--- a/src/decoder/_flac_common.c
+++ b/src/decoder/_flac_common.c
@@ -345,3 +345,97 @@ flac_common_write(struct flac_data *data, const
FLAC__Frame * frame,
return FLAC__STREAM_DECODER_WRITE_STATUS_CONTINUE;
}
+
+char*
+flac_cue_track(const char* pathname,
+ const uint8_t tnum)
+{
+ FLAC__StreamMetadata* cs =
FLAC__metadata_object_new(FLAC__METADATA_TYPE_CUESHEET);
+
+ FLAC__metadata_get_cuesheet(pathname, cs);
+
+ if (cs == NULL)
+ return NULL;
+
+ if (cs-data.cue_sheet.num_tracks = 1)
+ {
+ FLAC__metadata_object_delete(cs);
+ return NULL;
+ }
+
+ if (tnum 0 tnum cs-data.cue_sheet.num_tracks)
+ {
+ char* track = g_malloc(sizeof(char));
+ if (tnum 10)
+ sprintf(track, track_00%ld.flac, (long)tnum);
+
+ else if (tnum = 10)
+ sprintf(track, track_0%ld.flac, (long)tnum);
+
+ else if (tnum = 100)
+ sprintf(track, track_%ld.flac, (long)tnum);
+
+ FLAC__metadata_object_delete(cs);
+
+ return track;
+ }
+ else
+ {
+ FLAC__metadata_object_delete(cs);
+ return NULL;
+ }
+}
+
+const char*
+flac_get_basename(const char* pathname)
+{
+ uint8_t tnum = 0;
+ char* ptr = NULL;
+ char* ptr_ = NULL;
+ char* ptrdot = NULL;
+ const char* filename;
+
+ FLAC__StreamMetadata* cs;
+
+ cs = FLAC__metadata_object_new(FLAC__METADATA_TYPE_CUESHEET);
+
+ FLAC__metadata_get_cuesheet(pathname, cs);
+
+ filename = strdup(pathname);
+
+ if (cs == NULL)
+ {
+ // last '/' in pathname: ++ptr should now start at
'track_xxx.flac'
+ ptr = strrchr(filename, '/');
+ *ptr = '\0';
+
+ // underscore in 'track_xxx.flac'; ++ptr_ = 'xxx.flac'
+ ptr_ = strrchr(++ptr, '_');
+ *ptr_ = '\0';
+
+ // ++ptrdot should start at 'flac'
+ ptrdot = strrchr(++ptr_, '.');
+ *ptrdot = '\0';
+
+ tnum = strtol(ptr_, NULL, 10);
+
+ if (strcmp(ptr, track) == 0
+ strcmp(++ptrdot, flac) == 0
+ tnum 0 tnum 255)
+ {
+ g_debug(return filename: %s, filename);
+ return filename;
+ }
+ else
+ return NULL;
+ }
+ else
+ return pathname;
+}
+
+void
+flac_cue_tag_song(struct tag* tag, const uint8_t tnum)
+{
+ //tag = NULL;
+ //tnum = 0;
+}
diff --git a/src/decoder/_flac_common.h b/src/decoder/_flac_common.h
index e876c35..cbbc496 100644
--- a/src/decoder/_flac_common.h
+++ b/src/decoder/_flac_common.h
@@ -175,4 +175,17 @@ FLAC__StreamDecoderWriteStatus
flac_common_write(struct flac_data *data, const FLAC__Frame * frame,
const FLAC__int32 *const buf[]);
+char*
+flac_cue_track(const char* pathname,
+ const uint8_t tnum);
+
+const char*
+flac_get_basename(const char* pathname);
+
+void
+flac_check_cue_track(struct tag* tag, const uint8_t tnum);
+
+void
+flac_cue_tag_song(struct tag* tag, const uint8_t tnum);
+
#endif /* _FLAC_COMMON_H */
diff --git a/src/decoder/flac_plugin.c b/src/decoder/flac_plugin.c
index ce056d2..6a44479 100644
--- a/src/decoder/flac_plugin.c
+++ b/src/decoder/flac_plugin.c
@@ -382,6 +382,233 @@ flac_decode(struct decoder * decoder, struct input_stream
*input_stream)
#ifndef HAVE_OGGFLAC
+/*
+ * @brief Open a flac file for decoding
+ * @param fname filename on fs
