tinytag icon indicating copy to clipboard operation
tinytag copied to clipboard

APE Format Support

Open Ace-Radom opened this issue 2 years ago • 11 comments

Hey there: I forked this repo and added supports for ape format. I'm not sure if you're insterested in it. The things I changed are:

  1. add .ape tag in SUPPORTED_FILE_EXTENSIONS list

    https://github.com/Ace-Radom/tinytag/blob/ef28a83e332a0ab5a51045defd940d72d0b7389d/tinytag/tinytag.py#L87

  2. add .ape pointing to class APE under cls._file_extension_mapping in function _get_parser_for_filename

    https://github.com/Ace-Radom/tinytag/blob/ef28a83e332a0ab5a51045defd940d72d0b7389d/tinytag/tinytag.py#L149

  3. add regex for ape format under cls._magic_bytes_mapping in function _get_parser_for_file_handle

    https://github.com/Ace-Radom/tinytag/blob/ef28a83e332a0ab5a51045defd940d72d0b7389d/tinytag/tinytag.py#L181

  4. then the APE class

I didn't add any new tests because I don't know how to do it. But I tested it with my own samples and it worked as it should be.

The only thing is the keynames of apetag is not defined by the developers of the format. That means: there can be different keynames for one field (e.g. artist). I wrote a map like:

apetag_mapping = {
    'title':        'title',
    'artist':       'artist',
    'album':        'album',
    'album artist': 'albumartist',
    'comment':      'comment',
    'composer':     'composer',
    'disc':         'disc',
    'discnumber':   'disc',
    'genre':        'genre',
    'track':        'track',
    'tracknumber':  'track',
    'year':         'year'
}

This covered all situations I had with the samples created by foobar2k and mp3tag. But like I said, there can be different keynames for one field, and I don't know how efficient this solution is.

Ace-Radom avatar Nov 23 '23 10:11 Ace-Radom

Thanks for your contribution! Support for APE files would be great.

You can add sample files here: https://github.com/devsnd/tinytag/tree/master/tinytag/tests/samples

Then add the metadata to the testfiles dictionary: https://github.com/devsnd/tinytag/blob/master/tinytag/tests/test_all.py

Ideally, test files should be as small as possible, and either contain silence, or be truncated to the point that almost no audio data remains. If possible, try to use e.g. Audacity to generate silent audio files, and then add metadata to them.

mathiascode avatar Nov 24 '23 14:11 mathiascode

Thanks for your contribution! Support for APE files would be great.

You can add sample files here: https://github.com/devsnd/tinytag/tree/master/tinytag/tests/samples

Then add the metadata to the testfiles dictionary: https://github.com/devsnd/tinytag/blob/master/tinytag/tests/test_all.py

Ideally, test files should be as small as possible, and either contain silence, or be truncated to the point that almost no audio data remains. If possible, try to use e.g. Audacity to generate silent audio files, and then add metadata to them.

Alright I'll try to build some of them.

If possible, please move any code before this to _determine_duration(), so tags aren't populated when tag parsing is disabled. If it's not possible, please check the _parse_tags and _parse_duration attributes for when to populate tags.

Ah okay, I wrote this part of the program with the reference to the Aiff._determine_duration and therefore I put all things into _parse_tag function, but I'll change it this night.

And one further thing, I realize that the checks are failing due to E501 line too long. Is there still something I need to change in the code? (cuz I never see such an error before)

Ace-Radom avatar Nov 26 '23 15:11 Ace-Radom

And one further thing, I realize that the checks are failing due to E501 line too long. Is there still something I need to change in the code? (cuz I never see such an error before)

Wrap the long lines so they don't exceed the limit (100 characters per line). Run flake8 locally to verify that everything is fine.

mathiascode avatar Nov 26 '23 16:11 mathiascode

I added one sample to the tests. Sadly I failed to find or build a sample with ape file format lower than version 3.98 (even with the offical encoder released in 2011), it's simply too old.

Ace-Radom avatar Nov 28 '23 12:11 Ace-Radom

I added one sample to the tests. Sadly I failed to find or build a sample with ape file format lower than version 3.98 (even with the offical encoder released in 2011), it's simply too old.

No problem, I could see if I can find some sample files. If you fix the remaining linting error, we can get a coverage report showing how much of the code is covered by the tests.

mathiascode avatar Nov 29 '23 23:11 mathiascode

Coverage Status

coverage: 95.221% (-1.3%) from 96.565% when pulling 92ca4848bf05209404ffe6a204a70d79699d8c9c on Ace-Radom:master into d8a62a2d32f6a1163ed3ef66a84a1967c8c6ab2a on devsnd:master.

coveralls avatar Nov 30 '23 10:11 coveralls

Hm doesn't look really fine (I mean the coverage) Is there still something I need to do?

Ace-Radom avatar Dec 01 '23 08:12 Ace-Radom

Here are audio files with both APEv1 and APEv2 tags. Could you add them to the sample files and ensure they are read properly? ape_tracks.zip

I'll try to generate a file for fver < 3980 too.

mathiascode avatar Dec 04 '23 16:12 mathiascode

Please also add a test_invalid_ape_file test (see test_all.py for previous examples), and extend the test_detect_magic_headers test.

Is there still something I need to do?

Ideally, we would have enough sample files so the tests cover every new line of code except self._parse_header(fh).

mathiascode avatar Dec 04 '23 16:12 mathiascode

Sorry for the really slow response, I was full buzy with something else in the past two weeks...

Here are audio files with both APEv1 and APEv2 tags. Could you add them to the sample files and ensure they are read properly?

I see, I'll do that tomorrow morning.

Please also add a test_invalid_ape_file test (see test_all.py for previous examples), and extend the test_detect_magic_headers test.

Okay I'll try to do that.

Ace-Radom avatar Dec 17 '23 21:12 Ace-Radom

No worries, thank you for working on this. :)

mathiascode avatar Dec 18 '23 16:12 mathiascode