Request for a -quiet flag

[brettsheleski]

Request for a -quiet flag

[BrettSheleski]

Basically there's just two parts that would need to be quieted.

Comskip echos some stuff out to stdout and i dont think it's as simple as adding a command line argument to silence that. I think that is controlled via its ini file (I dont want to touch the ini file more than needed). Perhaps redirecting comskip's stdout to /dev/null should work there.

ffmpeg also is quite chatty. According to its documentation the -loglevel panic can be added along with -hide_banner to only output to stderr on actual error.

I like the idea of comchap silently succeeding, and noisily failing. I'm now of the opinion that these two options could be done by default in the script without needing a --quiet option. Instead, if more info is needed a --verbose option could be later implemented essentially undoing what is proposed here.

Thoughts?

[Reed97123]

Yep, I'd agree with most of what you have above.

Comskip has this argument (that I haven't tried yet):

  -q, --quiet               Not output logging to the console window

With ffmpeg in my current post-processing items I resorted to using ">/dev/null 2>&1" at the end of my ffmpeg command and then I simply check to see if there's an output file to see if it completed correctly. However, I wonder if the -loglevel panic and -hide_banner might not be a more eloquent approach.

In my post-processing I operate as you mentioned above, when I am debugging I use a -verbose switch. When I'm done debugging I remove the -verbose and it only outputs when there is an error, which is then e-mailed to me when run through cron.

As a random side-note, this doesn't affect functionality but you can (very) slightly simplify your ffmpeg command using the single -codec copy which I noticed in the ffmpeg documentation:
https://www.ffmpeg.org/ffmpeg-formats.html

ffmpeg -i INPUT -i FFMETADATAFILE -map_metadata 1 -codec copy OUTPUT

[BrettSheleski]

I just compiled Comskip from github source on an Ubuntu Virtual Machine and there doesn't appear to be a -q nor --quiet option implemented.

Also, redirecting stderr to /dev/null doesn't make comskip silent, nor does redirecting stdout. However if I do a &>/dev/null it is indeed silent. I don't like needing to do this, but I don't see a better alternative at this point.

As for your note about -codec copy. I think I originally did that at one point, but for some reason changed it. I just committed a change to -codec copy. Again, untested, enjoy :)

[Reed97123]

I can confirm your findings. Using the https://github.com/erikkaashoek/Comskip version (which if I recall is the one you are using), it lists -q as an option, it ignores it if you use it. It was probably part of the original code (since it doesn't give an error), but subsequent modifications to the code probably didn't utilize it. So piping both stderr and stdout to /dev/null is probably the only way to make it quiet.

Thanks for making the changes. I'll give it a nice thorough testing after work this evening.

[Reed97123]

I made some edits to make it silent. I used the following method found on the internet:

#!/bin/bash
verbose=0

exec 3>&1
exec 4>&2

if ((verbose)); then
  echo "verbose=1"
else
  echo "verbose=0"
  exec 1>/dev/null
  exec 2>/dev/null
fi

echo "this should be seen if verbose"
echo "this should always be seen" 1>&3 2>&4

Then add 1>&3 2>&4 only to commands of which you want to see the output.

I'll put the new code into the next comment. Feel free to use/disregard according to your preference.

[Reed97123]

#!/usr/bin/env bash

#LD_LIBRARY_PATH is set and will mess up ffmpeg, unset it, then re-set it when done
ldPath=${LD_LIBRARY_PATH}
unset LD_LIBRARY_PATH

exitcode=0

ffmpegPath="ffmpeg"
comskipPath="comskip"

# Setup for verbose
exec 3>&1
exec 4>&2

if [[ $# -lt 1 ]]; then

  exename=$(basename "$0")

  echo "Add chapters to video file using EDL file" 1>&3 2>&4
  echo "     (If no EDL file is found, comskip will be used to generate one)" 1>&3 2>&4
  echo "" 1>&3 2>&4
  echo "Usage: $exename infile [outfile]" 1>&3 2>&4

  exit 1
fi

comskipini=$HOME/.comskip.ini

deleteedl=true
deletemeta=true
deletelog=true
deletelogo=true
deletetxt=true
verbose=false
lockfile=""

while [[ $# -gt 1 ]]
do
key="$1"
case $key in
    --keep-edl)
    deleteedl=false
    shift
    ;;
    --keep-meta)
    deletemeta=false
    shift
    ;;
    --ffmpeg=*)
    ffmpegPath="${key#*=}"
    shift
    ;;
    --comskip=*)
    comskipPath="${key#*=}"
    shift
    ;;
    --comskip-ini=*)
    comskipini="${key#*=}"
    shift
    ;;
    --lockfile=*)
    lockfile="${key#*=}"
    shift
    ;;
    --verbose)
    verbose=true
    shift
    ;;
    *)
    break
    ;;
esac
done

if [ ! -z "$lockfile" ]; then

  echo "lockfile: $lockfile"
  while [[ -f "$lockfile" ]]; do
    echo "Waiting" 1>&3 2>&4
    sleep 5
  done

  touch "$lockfile"
fi

if [ ! -f "$comskipini" ]; then
  echo "output_edl" > "$comskipini"
elif ! grep -q "output_edl=1" "$comskipini"; then
  echo "output_edl=1" >> "$comskipini"
fi

if ! $verbose; then
  exec 1>/dev/null
  exec 2>/dev/null
fi

infile=$1
outfile=$infile

if [[ -z "$2" ]]; then
  outfile="$infile"
else
  outfile="$2"
fi

outdir=$(dirname "$outfile")

outextension="${outfile##*.}"

edlfile="${infile%.*}.edl"
metafile="${infile%.*}.ffmeta"
logfile="${infile%.*}.log"
logofile="${infile%.*}.logo.txt"
txtfile="${infile%.*}.txt"

if [ ! -f "$edlfile" ]; then
  $comskipPath --ini="$comskipini" "$infile"
fi

start=0
i=0
hascommercials=false

echo ";FFMETADATA1" > "$metafile"
# Reads in from $edlfile, see end of loop.
while IFS=$'\t' read -r -a line
do
  ((i++))

  end="${line[0]}"
  startnext="${line[1]}"
  hascommercials=true

  echo [CHAPTER] >> "$metafile"
  echo TIMEBASE=1/1 >> "$metafile"
  echo START="$start" >> "$metafile"
  echo END="$end" >> "$metafile"
  echo "title=Chapter $i" >> "$metafile"

  echo [CHAPTER] >> "$metafile"
  echo TIMEBASE=1/1 >> "$metafile"
  echo START="$end" >> "$metafile"
  echo END="$startnext" >> "$metafile"
  echo "title=Commercial $i" >> "$metafile"

  start=$startnext
done < "$edlfile"

if $hascommercials ; then
  ((i++))
  echo [CHAPTER] >> "$metafile"
  echo TIMEBASE=1/1 >> "$metafile"
  echo START="$start" >> "$metafile"
  echo "title=Chapter $i" >> "$metafile"

  if [ "$infile" -ef "$outfile" ] ; then

    tempfile=`mktemp --suffix=."$outextension" "$outdir"/XXXXXXXX`

    echo Writing file to temporary file: "$tempfile"
    if $ffmpegPath -nostdin -i "$infile" -i "$metafile" -map_metadata 1 -codec copy -y "$tempfile"; then
      mv -f "$tempfile" "$outfile"
      echo Saved to: "$outfile"
    else
      exitcode=-1
    fi
  else
    if $ffmpegPath -nostdin -i "$infile" -i "$metafile" -map_metadata 1 -codec copy -y "$outfile"; then
      echo Saved to: "$outfile"
    else
      exitcode=-1
    fi
  fi
elif [ ! "$infile" -ef "$outfile" ] ; then

    if $ffmpegPath -nostdin -i "$infile" -codec copy -y "$outfile"; then
      echo Saved to: "$outfile"
    else
      exitcode=-1
    fi
else
  echo No Commercials found, and not saving to new location.  Nothing to do.
fi

if $deleteedl ; then
  if [ -f "$edlfile" ] ; then
    rm "$edlfile";
  fi
fi

if $deletemeta ; then
  if [ -f "$metafile" ]; then
    rm "$metafile";
  fi
fi

if $deletelog ; then
  if [ -f "$logfile" ]; then
    rm "$logfile";
  fi
fi

if $deletelogo ; then
  if [ -f "$logofile" ]; then
    rm "$logofile";
  fi
fi

if $deletetxt ; then
  if [ -f "$txtfile" ]; then
    rm "$txtfile";
  fi
fi

if [ ! -z $ldPath ] ; then
  #re-set LD_LIBRARY_PATH
  export LD_LIBRARY_PATH="$ldPath"
fi

if [ ! -z "$lockfile" ]; then
  rm "$lockfile"
fi

[BrettSheleski]

That should work. Commit coming shortly.

[BrettSheleski]

See added commit.

One thing I already don't like is if you specify a bad input file name it silently does nothing (unless --verbose is specified).

I may make another commit to address situations like this.

[BrettSheleski]

Check it out now. I'm still not totally satisfied with using redirection to added file descriptors, but it works.

I may refactor to use a more elegant solution at some other point in the future.

[Reed97123]

Tested. Looks perfect for me.

I agree some input file testing wouldn't be amiss, but this is a lower priority for me since once I have this hooked in properly with my other scripts I shouldn't be passing in any incorrect file names.