digimezzo
commented
1 year ago Thank you so much for putting the effort add support for MP4 files! I'm beyond thrilled that someone was interested in contributing to little side project of mine. Full disclosure, I may not have the struck the right balance of pedantic vs important in these comments. Code reviews in a professional scenario, like I do day-to-day, probably need a different level of scrutiny than a volunteer pull request. As such, feel free to address or ignore any of these comments - the tests verify it works, and that's the most important part. I'll likely tweak some of it as well before it gets released (please don't take that personally!) Like I said, I am incredibly grateful you wanted to contribute back!
You're very welcome. I'm using your library in one of my projects and I needed m4a support. So I decided to try porting it. Thank you for the thorough review. Don't worry about the pedantic vs important ratio. I spend time at both sides of pull requests (development and review) in a professional scenario on a daily basis. And I much prefer very thorough reviews of my code. I'll go through your comments and will address them in the next days. Don't worry if I go silent for a few days next week. I'll take a short vacation away from computers. But I'll definitely address all your comments.
benrr101
ion-dev
This pull request adds mpeg4 support and fixes #67. It is a complete translation of the C# TagLib# implementation.
Note: due to the inheritance structure of the mpeg4 boxes in the C# implementation, I was forced to put all box classes into a single file mpeg4boxes.ts. The boxes depend on each other in a circular way (e.g.: Mpeg4Box imports IsoHandlerBox while IsoHandlerBox extends Mpeg4Box and thus imports Mpeg4Box), which causes the error "Cannot access Mpeg4Box before initialization" in runtime if the boxes are each placed in their own file. I've put this info as a class comment in file mpeg4boxes.ts.