TheCaptain989 / radarr-striptracks

A Docker Mod to Radarr/Sonarr to automatically strip out unwanted audio and subtitle tracks
https://hub.docker.com/r/thecaptain989/radarr-striptracks
GNU General Public License v3.0
20 stars 1 forks source link

Write new videofile to tempfile first and rename after striptracks completion #46

Closed tam1m closed 1 year ago

tam1m commented 1 year ago

Why? Because Jellyfin for example finds and scans (ffprobe) the videofile while being written by striptracks. FFprobe outputs wrong data because the file is still incomplete while being probed. This often results in ffprobe outputting a videolength of 0 for example.

How to prevent this? My proposal would be to simply write the new file to a temp file that does not have a valid videofile extenstion that's would be picked up by mediaservers and rename it only AFTER striptracks completeted its tasks. This way, the videofile is only ever being probed by other software in a complete and readable state.

TheCaptain989 commented 1 year ago

Thanks for the suggestion. I remember making the decision early on to write the new file with its final name to avoid problems, but I can't recall what those problems were! 🤪 Please give me some time to review my own code and try to figure out what I was thinking and then I'll get back with you.

TheCaptain989 commented 1 year ago

Well, I've read through everything twice and I can't for the life of me remember why I renamed the original video first. :stuck_out_tongue_closed_eyes: I'll start reworking it and testing and let you know.

TheCaptain989 commented 1 year ago

I did find why I did this in the first place: Issue #18 However, that was 3 years ago with old versions of Radarr/Sonarr and I can't reproduce the behavior anymore with the current versions. So, I've just made your suggested change and did some smoke testing, but that's it.

Please @tam1m test this and let me know how it works for you. Change your DOCKER_MODS variable to DOCKER_MODS=thecaptain989/radarr-striptracks:latest and let me know how it goes.

tam1m commented 1 year ago

Thank you. I'll test this over the next couple of days and will report back.

tam1m commented 1 year ago

So, I've tested this a couple of times now and it seems to be working fine. I'd assume, now with striptracks writing the temp file, there is still the possibility of jellyfin picking up the original file to fast, where it then gets replaced by the new file while still being analyzed. But the time window for that to happen should be a lot smaller, the original file is atleast readble by ffprobe and I've not encountered this in my couple of tests. So all seem good so far.

Optimally, sonarr/radarr should have a pre-import hook, where file modifications can be done before the file is being placed in it's destination directory, but thats not a striptracks problem. I guess, another way would be renaming the original file aswell, so both the modified and the original file wouldn't get recognized as media but that seems hacky and might interfere with other tools accessing the file at the same time. So, I think this can be closed for now.

Thank you @TheCaptain989

tam1m commented 1 year ago

After 10 days of testing I've yet to run into any issues with this. I also tried and could not reproduce the issue mentioned in https://github.com/TheCaptain989/radarr-striptracks/issues/18#issue-479276505.

TheCaptain989 commented 1 year ago

Thanks for your help! My own testing and experience agrees with yours.

I'll open a PR to move to prod.