search

Godzil / Crunchy

Crunchy is capable of downloading anime episodes from the popular CrunchyRoll streaming service.
MIT License
95 stars 19 forks source link

changed the merge from using mkvmerge to using ffmpeg #118

Open elisha464 opened 4 years ago

elisha464 commented 4 years ago

when merging the subtitles with the video mkvmerge was sometimes throwing warnings, and when it throws a warning the exit code of the program is 1 and that throws an error even though the merge is totally OK I switched the merge to use ffmpeg, it feels much more suited for the job and it also removes the dependency on another executable

Checklist

[X] I've run npm run compile and it produce no error [X] I've run npm run test and it produce no error [X] I've not pushing more than one feature in that pull request [X] My branch is updated with the latest from main when I make that pull request [X] I've tested as much as I can my changes

codeclimate[bot] commented 4 years ago

Code Climate has analyzed commit a45f12b2 and detected 0 issues on this pull request.

View more on Code Climate.

Godzil commented 4 years ago

The merge is not OK. Some episodes have incorrect subtitles, and they need to be corrected. Yes mkvmerge return with an error value for a warning; but the subtitles are really incorrect.

There is an open issue about that: #65, and instead of using a tool which does not warn/errored on invalid subtitles (some of the entries are beyond the length of the video) I would prefer to have a way to detect them and patch the file on the fly.

elisha464 commented 4 years ago

are you sure that the problem is because of the subtitles? and if so what kind of fix did you have in mind?