jansmolders86 / mediacenterjs

A HTML/CSS/Javascript (NodeJS) based Media center
http://mediacenterjs.com
1.29k stars 243 forks source link

Scrapers getting things wrong #156

Closed CodingPeak closed 9 years ago

CodingPeak commented 9 years ago

Movie scraper: Most of the movies are not recognized (no image, synopsis or length. Only a title...) Movie structure used:

Movies

Series scraper: I have a directory with series (all folders), but only the last folder is analyzed. Within that folder, I have 5 folders (each a season). Only the 3rd, 4th and 5th folder are analyzed. Within the 3rd folder, only files 5 to 13 are analyzed... I already checked the format that's documented here, but I think there's nothing wrong with that... Series structure used:

Series

Music scraper: For some reason, many tracks are put together in an album. Haven't yet figured out why this is happening... Suggestion: random play across all tracks (now only within an album).

Jon889 commented 9 years ago

Can you give an example of a movie name that isn't recognised? You might have to manually edit some of the names for the metadata fetcher to pick up the right movie (for example if you have extra words such as the quality of the movie) and then click Fetch metadata in the edit dialog.

I'm not sure about the TV shows, as I don't really have any on my computer to test with.

For the music, do you mean tracks should be in separate albums are being added to the same album?

CodingPeak commented 9 years ago

There are many examples... But I should first reinstall MCJS, as my antivirus deleted the .exe file --'. Be right back with answer.

CodingPeak commented 9 years ago

Jon889, can I send you a private message?

Jon889 commented 9 years ago

yes, my email is jbpdeveloper@me.com

Jon889 commented 9 years ago

So when fetching metadata it should return multiple movies so the user can choose if it returns the wrong one. Also parsing names that haven't been cleaned up should probably be a feature.

CodingPeak commented 9 years ago

Why not Levenshtein the movie names to get the best match? http://en.wikipedia.org/wiki/Levenshtein_distance

Jon889 commented 9 years ago

I'm not really sure that would work in this case? Because if TMDB had a result for Up as a perfect match it seems like it should have been matched.

CodingPeak commented 9 years ago

Oh, you don't get a list of possible matches of TMDB?

Jon889 commented 9 years ago

I wasn't sure. I've just checked and it does return a list, however there are 2 movies called "Up" and one called "Up!" so even if it is determined to be a perfect match, it can still find the wrong movie.

CodingPeak commented 9 years ago

Well, https://www.themoviedb.org/search/movie?query=up this is propably the result you’re getting… But I guess you don’t get the years? Because that would make a huge difference..

Jon889 commented 9 years ago

Yes, the year should be included already, I'll see why it isn't. Still the option to select from that list would be good.

Jon889 commented 9 years ago

The year wasnt saved to the database, so there was no to update it using the year. I've added a year field, so editing the movie with a title of Up and a year of 2009 returns the correct movie

jansmolders86 commented 9 years ago

Great job Jonathan! You're a life saver!

CodingPeak commented 9 years ago

Uhm, problem about the series isn't solved yet I guess... ?

jansmolders86 commented 9 years ago

Hey @Jon889 ,

I seem to get this error after a clean install:

First run, Setup running on localhost:3000

Executing (default): CREATE TABLE IF NOT EXISTS `SequelizeMeta` (`id` INTEGER PRIMARY KEY AUTOINCREMENT, `from` VARCHAR(255), `to` VARCHAR(255));
Executing (default): SELECT * FROM `SequelizeMeta` ORDER BY id DESC LIMIT 1;
Running migrations...
20141022122234-movieyear.js
Executing (default): ALTER TABLE `Movies` ADD `year` INTEGER;
Executing (default): INSERT INTO `SequelizeMeta` (`id`,`from`,`to`) VALUES (NULL,20141022122234,20141022122234);

C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sqlite3\lib\trace.js:28
                    throw err;
                          ^
Error: SQLITE_ERROR: duplicate column name: year
--> in Database#all('ALTER TABLE `Movies` ADD `year` INTEGER;', [Function])
    at executeSql (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\dialects\sqlite\query.js:43:54)
    at Database.<anonymous> (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\dialects\sqlite\query.js:67:11)
    at module.exports.Query.run (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\dialects\sqlite\query.js:34:19)
    at module.exports.ConnectorManager.query (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\dialects\sqlite\connector-manager
.js:44:70)
    at TransactionManager.query (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\transaction-manager.js:52:49)
    at module.exports.Sequelize.query (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\sequelize.js:330:36)
    at null.<anonymous> (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\query-interface.js:971:40)
    at null.<anonymous> (C:\Users\Jan\Documents\GitHub\mediacenterjs\node_modules\sequelize\lib\emitters\custom-event-emitter.js:24:18)
    at processImmediate [as _immediateCallback] (timers.js:345:15)

Is this related to the year change by any chance?

Jon889 commented 9 years ago

Ah yes, I think I need to put that in the success callback for the sync function.

I've just made the change and committed it, I can't really test it as my internet is ridiculously slow to clone a fresh copy off github.

jansmolders86 commented 9 years ago

no problem! I'll test it! thanks for fixing it!

jansmolders86 commented 9 years ago

I'm afraid I'm still getting the error Jonathan. Is there anything I can do to help? I don't see why this occurs in the code.

jansmolders86 commented 9 years ago

@Jon889 To temporarily fix this issue I've commited a workaround.

I removed the database from the gitignore and added an empty database.

Of course this is far from ideal but new users will be able to get it running now. If you have any idea how to fix it I would love any input you can give. I have no clue to be honest.

Jon889 commented 9 years ago

We just got internet back yesterday, so I'll try it out now

jansmolders86 commented 9 years ago

No worries my friend :)

I feel a bit frustrated I couldn't figure it out myself. Anything you can do is great but only if it is convenient for you of course! .

Jon889 commented 9 years ago

Ah sorry, I misread the error the first time, I thought it was saying it couldn't migrate because there was no database, but it's trying to migrate when it doesn't need to. I'll ask at the sequelize repo.

jansmolders86 commented 9 years ago

Thanks Jon!

Jon889 commented 9 years ago

It seems they are changing the way migrations work. So is it ok to leave the temporary fix as it is? I could add a check in the code to not run the migrations if the sequelize metadata table is empty in the database, which I'm pretty sure would mean that an empty database wouldn't need to be distributed in a fresh install. But it would still be hacky.

jansmolders86 commented 9 years ago

I would love to have that check Jon! Currently it's pretty unsettling one could easily commit the database which is very undesirable. The check would remedy that and present a decent solution untill the dust settles in the sequelize camp. agreed?

th3l0g4n commented 9 years ago

I have created a fix for the DB error on startup, maybe you can have a look (#166).

Jon889 commented 9 years ago

Ah, sorry I didn't see you're pull request before I'd pushed a fix. I implemented the check in the code that runs the migration, which means the migrations can stay relatively simple and the check doesn't need to be added to each one, unless you can see an issue with doing it that way?

th3l0g4n commented 9 years ago

No Problem, could have said that i will also take a look :>

I think the SequelizeMeta Table is used to track previous upgrades through the automatic update procedure? This should be ok, but doing it that way you have to be sure that every column which will be added by a migration script is also present in the Model itself because they will not be added when not.

Maybe the SequelizeMeta Table could also be a Model to be consistent?

Jon889 commented 9 years ago

Yes, the database schema has to be the most up to date version, the migrations are for getting from an old version to the new version of the schema.

I don't think it can be because the SequelizeMeta table is used internally, so I think creating a definition would conflict with that?

th3l0g4n commented 9 years ago

I wouldn't think so, but if its working for now, it maybe shouldn't be changed.