benrr101 / node-taglib-sharp

A node.js port of mono/taglib-sharp
GNU Lesser General Public License v2.1
42 stars 11 forks source link

Support for M4A #67

Closed ion-dev closed 1 year ago

ion-dev commented 1 year ago

Hey, would also be great to have support for ".m4a"! Any plans to add this?

benrr101 commented 1 year ago

yes! I could've sworn that M4A was covered by the AAC work I did a long time ago, but I see that it does not. Right now I'm still ... slowly ... working on matroska/webm support. However, there is a user who's been working on contributing the MPEG4 implementation. I'm not sure where @digimezzo is on it (and no pressure, it's totally volunteer work that I greatly appreciate!), but according to the latest commit messages, it seems to be progressing smoothly.

digimezzo commented 1 year ago

Hi @ion-dev and @benrr101. I'm indeed working on MPEG4 support. The implementation is actually finished and manual testing confirms it works, but I still need to write automated tests. As soon as those are done, I'll submit a pull request to this repository.

digimezzo commented 1 year ago

Hi @ion-dev and @benrr101. I wanted to give an update on MPEG4 support because I've been silent for a long time. Between translating the C# implementation and the writing of tests I took a long break from it. I got caught up into another project of mine. But I am now again actively working on MPEG4 support. I'm currently writing automated tests based on the tests that are in the C# project. Unit tests are done. Integration tests for M4A are also written, however some still fail. Tests for reading and writing tags are successful, but some tests reading audio properties return values that are sligtly off (e.g. ReplayGain) or just wrong (e.g. AudioSampleRate). I'm checking out why. Once that's done, I'll write the M4V integration tests. I hope to be done in a few days, but that'll depend on the difficulty to find out why some values are off.

benrr101 commented 1 year ago

Hi @digimezzo That's great to hear! I've been taking a break from this project to work on some other projects, lately. So excited that you're still interested in contributing :D As always, feel free to reach out if you have any questions.

The issue you mentioned with audio properties in your tests, I'm curious whether it's a difference from the .NET implementation vs the node implementation. Through my testing for other formats, I've seen instances where the .NET implementation got things wrong. So now I usually confirm whatever values they have in their tests with a couple different players that give the full details (vlc and foobar2000, usually) and make sure my code matches their output. Either way, don't get too hung up on the minutia. If you get totally stuck on something, we can log it as a known issue and we can work together to come up with a solution.

ion-dev commented 1 year ago

Great to hear that @digimezzo !! Really excited to test it out! Top work guys

digimezzo commented 1 year ago

@ion-dev , @benrr101 Glad I could help :) It appears that all problems (ReplayGain, AudioSampleRate and others) were caused by small mistakes in my conversion to Typescript. The tests were very helpful in pinpointing those and I'm making good progress. I want to have a pull request ready before the end of this week.

Thanks for the tips @benrr101. It's good to know that the C# implementation is not flawless. I'll be sure to check what other players are giving back as values if I'm seeing explainable things.

digimezzo commented 1 year ago

HI @ion-dev , @benrr101. The pull request is ready for review :)