mediamicroservices / mm

MIT License
69 stars 27 forks source link

Linux tweaks #206

Open privatezero opened 6 years ago

privatezero commented 6 years ago

Made some changes to address problems in https://github.com/mediamicroservices/mm/issues/205 and make things a bit more cross platform.

Haven't been able to test in macOS, so probably should be run by someone with access to a mac!

In particular, I think this command change is cross platform, but should be confirmed!

privatezero commented 6 years ago

made some changes to make it more specific - I set it to warn rather than exit if it can't set these variables as they are only used in a small subset of scripts and I wouldn't want to cripple any scripts that potentially would work.

privatezero commented 6 years ago

@dericed , @aeschweik, could one of you confirm that switching to file -i INPUT | grep video away from file -Ib INPUT | grep video doesn't throw an error on macOS? (And gets a non null response when targeting a video file). :penguin: :penguin:

privatezero commented 6 years ago

had @ablwr test the file command on macOS, and it appears that it is in fact case sensitive. Will have to make another tweak before merging!

aeschweik commented 6 years ago

Sorry to have missed this earlier - it's working on a Mac over here! (with the change back to file -Ib)

privatezero commented 6 years ago

Thanks @aeschweik! Guess I'll merge next week if there are no other suggestions/objections?

dericed commented 6 years ago

Since homebrew symlinks xml to xmlstarlet, https://github.com/Homebrew/homebrew-core/blob/3147824bf1507ea3f7e6e8d9517d47f13f8838a7/Formula/xmlstarlet.rb#L22, I prefer the easier approach of substituting xmlstarlet for xml.

privatezero commented 6 years ago

Cool, that makes sense - since I couldn't test it myself I was using an abundance of caution - I'll switch it over to the universal command!

privatezero commented 6 years ago

Made more tweaks to the tweaks!

retokromer commented 6 years ago

isn't that because it is using command substitution and attempting to run the output of $(${VIDEOCHECK} /Users/daverice/Desktop/test5_prores.mov) as a command?

I guess so as well. On my end, Andrew’s version works on both Ubuntu 16.04.3 LTS and macOS 10.11.6.

dericed commented 6 years ago

After https://github.com/mediamicroservices/mm/pull/226/commits/530d66289309eed068e7f16519603ef43fd7b62e, I think what remains in this PR is a system-specific handling of the font files for ffmpeg and some workaround for macos's file -Ib vs linux's file -i.

For file handling I suggest to do it by file accessibility rather than by system, such as

if [[ -f "/Library/Fonts/somefont.tff" ]] ; then
  TIMECODE_FONT="/Library/Fonts/somefont.tff"
elif [[ -f "/usr/lib/somewhere/where/i/hide/the/fonts/on/linux/somefont.tff" ]] ; then
  TIMECODE_FONT="/usr/lib/somewhere/where/i/hide/the/fonts/on/linux/somefont.tff"
else
  _report "uhhh, where's your fonts?"
fi

and file -Ib can be replaced by ffmpeg with a duration_ts check and video stream check, such as done in https://github.com/mediamicroservices/mm/blob/7aa16253c20b52ed8ddcdbded971245984d9732e/mmfunctions#L999-L1006

privatezero commented 6 years ago

I think that both of those options sounds like reasonable changes to me!

retokromer commented 6 years ago

Could possibly the conflicts be resolved and this pull request merged? Thanks!