Skip to content

changed the merge from using mkvmerge to using ffmpeg#118

Open
elisha464 wants to merge 1 commit into
Godzil:masterfrom
elisha464:fix-merge
Open

changed the merge from using mkvmerge to using ffmpeg#118
elisha464 wants to merge 1 commit into
Godzil:masterfrom
elisha464:fix-merge

Conversation

@elisha464

Copy link
Copy Markdown

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

when merge 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 eventhough 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
@qlty-cloud-legacy

Copy link
Copy Markdown

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

View more on Code Climate.

@Godzil

Godzil commented Apr 10, 2020

Copy link
Copy Markdown
Owner

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

Copy link
Copy Markdown
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants