Re: [Musicpd-dev-team] Support for parsing/tagging flac files according to embedded cuesheets
On 2009/03/24 10:10, Max Kellermann m...@duempel.org wrote: Do you want a git repository for that? Is there a public svn repository, which we could import to get the full code history into git? Found it, and cloned: http://git.musicpd.org/cgit/mirror/cuetools.git/ Maybe we should ask if Svend has abandoned the project, in this case you could take over the whole project, and just change the build scripts to generate an additional shared library with a well-defined and stable API. Max -- Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are powering Web 2.0 with engaging, cross-platform capabilities. Quickly and easily build your RIAs with Flex Builder, the Eclipse(TM)based development software that enables intelligent coding and step-through debugging. Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Support for parsing/tagging flac files according to embedded cuesheets
Hi Max, Max Kellermann wrote: It's important to document what was written by the original author and what was modified by you. Media players are a dangerous area (regarding copyright and patents), and we need to have a reliable audit trail of everything, just in case. I understand that and agree with you on that issue. So what i've done is this: I've built a shared library based on cuetools. I want to re-release this stuff as libcue on sourceforge or whereever so we only need to install and link mpd against it. I already contacted Svend about this. If he is willing to let me relicence it as BSD software, i'll do so. Otherwise i'd have to go with GPL but that wouldn't hurt either i think. Now i'm reading about automake/autoconf so i have a build system for the initial/first/alpha release.. :) Enjoy your holidays. Regards, Jochen signature.asc Description: OpenPGP digital signature -- Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are powering Web 2.0 with engaging, cross-platform capabilities. Quickly and easily build your RIAs with Flex Builder, the Eclipse(TM)based development software that enables intelligent coding and step-through debugging. Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Support for parsing/tagging flac files according to embedded cuesheets
On 2009/03/20 18:19, Jochen Keil jochen.k...@gmail.com wrote: The clean way to do it would be to create one commit which adds the original sources, and another commit edits them and adds them to the build system (Makefile.am). That'll be some work.. It's important to document what was written by the original author and what was modified by you. Media players are a dangerous area (regarding copyright and patents), and we need to have a reliable audit trail of everything, just in case. What's this for? It's for the one who is interested to build the lexer/parser himself. You don't need it though, so leave it out if you want to. I have to admit that i don't know enough about automake/autoconf to add a build lexer/parser command or so.. If you don't manage to write it, I can help you with that (when I return from vacation). Who's the author? Why does it include cd.h? Why do we need cd.h at all? Looks like it's dealing with information for compact discs, but MPD doesn't handle CDs. The author of the original cuetools package is Svend Sorensen. cd.h contains the methods for accessing the struct which holds the information from the parsed cuesheet. Svend called that struct Cd, thus cd.h/.c. I have no problem with that but if you'd like a different name for it i can change that.. as i said i wanted to rework the api anyway. I don't care much about the name, I was just questioning if you added it for a good reason. Don't create a separate commit for each and every source file you import. cd.h is nothing without cd.c. Should i add them all at once or cd.h + cd.c, cue.h + cue.c, etc.? If you need all of them at once, import all of them (the original sources) in one commit (without Makefile.am), with correct author information. Then do another commit (the integration commit), which modifies them (fix warnings? do whatever is required to get them compiled cleanly with --enable-werror), and adds them to the Makefile.am. You're the author of this commit. If you need to do API changes, you can do it in the following commits. Each commit which adds a new source must add it to Makefile.am. Each commit must not fail during build. make distcheck should always work, too. So it should be like this: new file (e.g. cd.c) + add to Makefile.am = one commit? In short, yes. See above for more detailed explanation. For commiting will it suffice to set user.name and user.email to Svend's contact information? git commit --author=Foo Bar f...@bar.org With stgit, you can easily change that information at any time with stg edit. Should i also add the files from cuetools i removed (and remove them again from git)? No. Import only the sources you need for your work. If you need more at a later time, you can make another import commit. Max -- Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are powering Web 2.0 with engaging, cross-platform capabilities. Quickly and easily build your RIAs with Flex Builder, the Eclipse(TM)based development software that enables intelligent coding and step-through debugging. Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Support for parsing/tagging flac files according to embedded cuesheets
On 2009/03/18 18:36, Jochen Keil jochen.k...@gmail.com wrote: please let me know about anything i should change or i could do better. I've taken the parser from cuetools (you can find the outdated project files on berlios) and modified the interface a bit. I don't see what you modified. Did you add the original (unmodified) sources, or did you only add the versions which were already modified by you? The clean way to do it would be to create one commit which adds the original sources, and another commit edits them and adds them to the build system (Makefile.am). I think it needs to be cleaned up a bit more but for i'm fine with. It's not leaking memory anymore (at least i hope so) and i've added the possibilty to delete an allocated cd structure. Hope you like it, I don't like it (yet)... see my comments below. 67e65a7..0ce399a: commit 0ce399a344093caa24328971b51f74039597e749 Author: Jochen Keil jochen.k...@gmail.com Makefile for the lexer/parser BSD style only for the moment. It is easy to understand though if you want to compile the test program or generate the lexer/parser. What's this for? commit 755f16d9d0f09e11cea836e83d8ea47b002133f6 Author: Jochen Keil jochen.k...@gmail.com Quick and dirty test program for the cuesheet parser Will not be compiled by mpd make Why not? Why not make it a test program in ./test/? This should live in test/, not in src/cue_parser/. If you don't compile it, don't add it to the source tree. commit eebbd9a8043f301c312e3e6f63654c12314b4a70 Author: Jochen Keil jochen.k...@gmail.com The implementation details for cue.h Who's the author? Why does it include cd.h? Why do we need cd.h at all? Looks like it's dealing with information for compact discs, but MPD doesn't handle CDs. commit 6fd0fa62fac30a90f5031940a8948731abe66c5d Author: Jochen Keil jochen.k...@gmail.com Interface for tagging a song from a cuesheet commit ae70da3fbcbb0956e2a9c1b64aff5be0369c54a9 Author: Jochen Keil jochen.k...@gmail.com Implementation details for time.h commit 507560abf378e84b450cab93c7f4ed9fe65882c7 Author: Jochen Keil jochen.k...@gmail.com Methods for handling index times in cuesheet indices commit d05b0e8f44e7264d6e6021a7f6e56dec213c7414 Author: Jochen Keil jochen.k...@gmail.com Implemenation for cdtext.h commit bc2cce8ed891db6b68ece6ef158251024617a781 Author: Jochen Keil jochen.k...@gmail.com The CDText API and data struct commit 92dffa294d76ecccf273e66fccb40061d5bee2d7 Author: Jochen Keil jochen.k...@gmail.com The implementation of the methods in cd.h Don't create a separate commit for each and every source file you import. cd.h is nothing without cd.c. commit 41b7bcd0b4d8b5180fdf2fe70bb05a0d86adcac1 Author: Jochen Keil jochen.k...@gmail.com The struct and the API for the cuesheet parser. commit 45403bcd4bbc2d86fca9951e855c673df8d8a44a Author: Jochen Keil jochen.k...@gmail.com The Parser (grammar) H source file commit 0240445a630e6a1a520c2e0a13dc1f1081749c52 Author: Jochen Keil jochen.k...@gmail.com The Parser (grammar) C source file commit 411bd222dfb1643237f4458753055c663959054d Author: Jochen Keil jochen.k...@gmail.com The Lexer (Tokenizer) C source file See below for my comment on generated sources. commit 74a52f5c2336ddde6623dc38752f7ca1523f937a Author: Jochen Keil jochen.k...@gmail.com The Parser (grammar) source file commit 10f6fec9a7d816aac0a9f98ae2f4cac9751f0baf Author: Jochen Keil jochen.k...@gmail.com The Lexer (Tokenizer) source file You're not the author of those files, yet the git commit reflects your name. I believe this is important. Anyway, what's the license of that source code? commit d7d5ab14f3938b836d69d1124659e2fbb5ffeaf5 Author: Jochen Keil jochen.k...@gmail.com Support for tagging flac files according to their embedded cuesheet commit 58ec6181c807b53de6d5a0e07e550a48c503c2fd Author: Jochen Keil jochen.k...@gmail.com Added sources for the cuesheet parser This patch adds sources to Makefile.am which don't exist yet. This breaks the build on this patch - every commit should be buildable, or bisect won't work properly. Each commit which adds a new source must add it to Makefile.am. Each commit must not fail during build. make distcheck should always work, too. Generating the lexer/parser using lex/yacc or flex/bison isn't necessary every time for compiling. That way we don't have to depend on those programs. I disagree. *Always* generate those sources. CUE support should be optional, so if lex/yacc are not available, don't build CUE support. We should never have generated files in git. Having yet another dependency isn't comfortable for users, but with this excuse, we could integrate everything into MPD. MPD used to have libmad built-in. -- Apps
