Use GetID3 package from Composer#346
Conversation
|
When we tried to update this lib from |
|
I haven't seen a big difference (but I haven't profiled either). I'll check that. For sure the package is heavier (I guess it handles more format as well), but I don't think it's really a problem if a package is taking 200 kB instead of 100 kB (numbers are fake). If you don't have enough space for that on your server, I guess you'll have other problems 😁 About the cover, a simple optimization could be to check for the cover image first in the album dir? It would be faster for sure, I know it would for me since I always keep a |
|
Ok so I tested the import of my library (2597 files) with the old and the new GetID3 lib, using the command line (because it's easier to benchmark like that):
That's clearly much longer (x4), but from my perspective, it's not that bad since it only affects the import. It would surely have been nicer if it has been faster, but I think using a package that is easy to upgrade is more valuable that the added delay during the import. Maybe GetID3 v2 that is in preview at the moment tries to address that? PS: By using the command line, I saw that there was a problem with the new CakePHP path there, so I fixed it: 6e7572d. |
|
I updated the CHANGELOG as well -- the "Unreleased" section is a good practice I got from https://keepachangelog.com/, so that you don't have to browse all the commits when it's time to do a release. |
|
@lGuillaume124 I know you're in the new database refactor, but since this PR also fixes #343, would you consider merging it? @gs11 would you like to review it as well? |
|
As the « WIP » label was still here I thought you were still working on it. |
|
LGTM, great work! @MightyCreak |
|
Thanks guys! |
The goal of this PR is to use GetID3 package from Composer instead of the old package included right now, and fix #343.
Here's what I've done:
This might not be the right way of doing things between CakePHP and Composer, I don't know much about those. Feel free to tell me how it should be done if there's a better way.
By using Composer's GetID3 package, the old GetID3 package is no longer needed so I removed the
app/Vendordirectory (it's the vast majority of this PR content).Once that done, in order to have only one
Vendordirectory for all the Composer packages (i.e. CakePHP and GetID3), I changed Composer installation directory fromVendortoapp/Vendor.Then I added
james-heinrich/getid3incomposer.json(and removeext-gdsince it doesn't seem to be a valid Composer package).Then I included the
app/Vendor/autoload.phpinbootstrap.phpto be able to easily instantiate classes from theVendordirectory.And finally I changed
CAKE_CORE_INCLUDE_PATHto point toapp/Vendorand I removed theApp::importfromSongManager.php.