Linux tweaks

[privatezero]

Made some changes to address problems in #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!

[retokromer]

I suggest to use an explicit

elif [[ $(uname -s)" = "Linux" ]]

and something like

else
    echo "Error: Unsupported OS. Exiting."
    exit 1
fi

[privatezero]

That makes sense to me!

I just noticed one thing, on linux in windows uname -s returns Linux which I assume would cause this to still assign a bad file path for the text file in that environment.

@retokromer, I know you have been running more of these scripts in the linux on windows environment, have you run into any problems due to the lack of specificity in using uname?

[retokromer]

I cannot remember a specific problem, but I will check as soon as I am back at the lab.

[privatezero]

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]

@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). 🐧 🐧

[privatezero]

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!

[retokromer]

I would add also here an

else

with a warning or an error.

[privatezero]

Since it is just a check to see if the input is a video (and throw an error if it isn't) I thought it would be better to attempt to set the variable, but then to skip the video check if the variable was unset (See line 48).

This way, if the person was using an irregular system (but somehow everything else was working) with a valid input it would allow them to proceed.

[retokromer]

I understand. Would in this case an

unset VIDEOCHECK

before the line 43 be appropriate?

[privatezero]

good point! I'll add that in!

[retokromer]

LGTM

[aeschweik]

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

[privatezero]

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

[dericed]

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]

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]

Made more tweaks to the tweaks!

[retokromer]

LGTM

[dericed]

on mac running:
VIDEOCHECK='file -Ib' ; $(${VIDEOCHECK} /Users/daverice/Desktop/test5_prores.mov)

I get
-bash: video/quicktime;: No such file or directory

[privatezero]

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?

In the same way that just running $(echo "hello") will also throw an error as it attempts to execute hello?

On my end, using the command VIDEOCHECK='file -i' ; $(${VIDEOCHECK} input.mov) will also throw an error, but within the context of the script, or even something like VIDEOCHECK='file -i' ; var=$(${VIDEOCHECK} input.mov) ; echo "${var}" gives the expected output/behavior.

[retokromer]

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]

This breaks for me. Maybe using ffmpeg is safer.

file -i TEST.mov 
/Users/daverice/Desktop/TEST.mov: regular file

vs

file -Ib TEST.mov 
video/quicktime; charset=binary

[retokromer]

Maybe using ffmpeg is safer.

+1

[dericed]

After 530d662, 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

mm/mmfunctions

Lines 999 to 1006 in 7aa1625

	find "${1}/objects" -type f -size +0 "${OBJECTS_FIND_EXCLUSIONS[@]}" | while read file ; do
	streamcount=$(ffprobe -loglevel quiet "$file" -show_entries format=nb_streams -of default=nw=1:nk=1)
	duration_ts=$(ffprobe -loglevel quiet "$file" -show_entries stream=duration_ts -of default=nw=1:nk=1)
	if [[ "$streamcount" > 0 && "${duration_ts}" != 1 ]] ; then
	_report -d "Input file: $file"
	echo "$file" >> "${INPUTFILES}"
	fi
	done

[dericed]

comments in #206 (comment)

[privatezero]

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

[retokromer]

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