audio-metadata icon indicating copy to clipboard operation
audio-metadata copied to clipboard

FIX: Decoding done with iconv-lite, support for all idv3 text encoding types

Open peerbelt-build opened this issue 8 years ago • 1 comments

peerbelt-build avatar Feb 11 '17 20:02 peerbelt-build

I'm not opposed in theory to either adding more support for encoding or adding more ID3v2 tags. However, there are a few goals that this library aims for:

  1. Smallish file size
  2. Speed

1 means there can be no external dependencies (and also because this needs to run in the browser).

Also, there are a lot of ID3v2 tags that can be supported; I don't necessarily want this library to handle all of them (due to the file size issue). By default it handles the most common ones, which I think is enough. Things like official_internet_radio_station_url are overkill for this library. If someone wants to handle all of those things, there are other more complete libraries for that.

So in order for me to accept this pull request, you would need to do the following:

  1. Ensure there are no dependencies
  2. Remove the added ID3v2 tags
    • I could be persuaded to keep some of them in if there are good reasons for it
  3. Add lots of encoding tests
  4. Fix small coding style issues, e.g. if ( foo ) {... should be if (foo) {.... I know there's no style linter, but at least making the spacing consistent will be enough.

If you think that's stupid, I understand. This library has very specific goals that I want to adhere to that may not be in line with what you need. If you want to keep your fork with more robust tag parsing, let me know where you put it and I will be happy to put a link to it in the README.

tmont avatar Feb 12 '17 19:02 tmont