Solution I needed to use the -nostdin argument to ffmpeg. Thanks to TwilightKiddy for the answer.

I’m running this in a backgrounded container to convert level 5.2 h264 files down to level 5.1. It’s working as expected in terms of all the checks for when to convert or not. But after the first file it comes across to convert, it performs the conversion, writes the final converted -> ... messages, and then exits the loop with the finished run message. I run the container again it dutifully finds the next file and converts it and cleanly exits again. I’ve done loads of other shell script looping over files using the while read do <<< $(find ...) pattern (to handle file paths/names with spaces and never had this issue.

container command:

docker run -d --rm --name ffmpeg -v ${HOME}/.local/bin:/scripts -v /volume1/media:/media --entrypoint /scripts/reduce_level.sh linuxserver/ffmpeg:latest -s /media/Movies -l /media/reduce_level.log  

script

#!/usr/bin/env bash  
# Find all .mp4 and .mkv files with h264 video streams whose level > 5.1 (ffprobe reports levels like 51 or 5.1)  
# and re-encode the video to level 5.1 keeping the same profile (if available).  
# Audio and subtitle streams are passed through (copied).  
#  
# Usage:  
#   ./reduce_level.sh         # actually convert  
#   ./reduce_level.sh --dry-run  # show the ffmpeg commands without running them  
#  
# Requirements: ffprobe and ffmpeg in PATH.  

set -euo pipefail  

log() {  
    local log_level=$1  # A string representing the log level provided by the user when calling the function  
    local message=$2  # A string representing the message provided by the user when calling the function  
    local script_name=$(basename $0)  # The name of the script that is running  
    local timestamp=$(date +"%Y-%m-%d %H:%M:%S")  # The current date and time at the time the function is called  
    if (( VERBOSE )); then  
      echo -e "$timestamp [$log_level] [$script_name] $message" | tee -a "${LOGFILE}"  
    else  
      echo -e "$timestamp [$log_level] [$script_name] $message" >> "${LOGFILE}"  
    fi  
}  

PARSED_ARGS=$(getopt -o dl:s:v --long dry-run,log-file:,start-dir:,verbose -- "$@")  
eval set -- "${PARSED_ARGS}"  

# Will be overwridden if arguments provided  
DRY_RUN=0  
VERBOSE=0  

while true; do  
  case "$1" in 
    -d | --dry-run )  
      DRY_RUN=1  
      shift  
      ;;  
    -l | --log-file )  
      LOGFILE="$(realpath "$2")"  
      shift 2  
      ;;  
    -s | --start-dir )  
      START_DIR="$(realpath "$2")"  
      shift 2  
      ;;  
    -v | --verbose )  
      VERBOSE=1  
      shift  
      ;;  
    -- )  
      shift  
      break  
      ;;  
    * )  
      log ERROR "Unsupported argument $1"  
      exit 1  
      ;;  
  esac  
done  

# Defaults if arguments not provided  
START_DIR="${START_DIR:-$(pwd)}"  
LOGFILE="${LOGFILE:-${START_DIR}/$(basename "${0/.sh/.log}")}"  

# Verify inputs  
[[ ! -d "${START_DIR}" ]] && { log ERROR "Could not find starting directory \"${START_DIR}\""; exit 1; }  
[[ ! -d "$(dirname "${LOGFILE}")" ]] && { log ERROR "Could not find log file directory \"$(dirname "${LOGFILE}")\""; exit 1; }  

log INFO "\n\n---------------- Starting run ----------------\n\n"  

while read f; do {  
  log INFO "Checking: $f"  

  # Get codec_name, profile and level for each video stream as separate lines (triplets)  
  probe_output=$(ffprobe -v error -show_format -show_streams -select_streams v -of json "$f")  

  if [[ "{}" == "$(jq -r '.' <<< ${probe_output})" ]]; then  
    log INFO "  no video streams found or ffprobe failed"  
    continue  
  fi  
  
  if [[ 1 -lt $(jq -r '.streams | length' <<< ${probe_output}) ]]; then  
    log WARN "  more than one video stream found, skipping"  
    continue  
  fi  

  need_convert=0  
  picked_profile=""  

  # Iterate over the probe output three lines at a time (codec, profile, level)  
  # We use a here-string so the loop runs in the current shell (no subshell) and doesn't need mapfile/readarray.  
  bit_rate_bps=$(jq -r '.format.bit_rate' <<< ${probe_output})  
  bit_rate_mbps=$(awk -v br=${bit_rate_bps} 'BEGIN{printf("%2.2f", br/1000000)}')  
  codec=$(jq -r '.streams[0].codec_name' <<< ${probe_output})  
  height=$(jq -r '.streams[0].height' <<< ${probe_output})  
  level_raw=$(jq -r '.streams[0].level' <<< ${probe_output})  
  profile=$(jq -r '.streams[0].profile' <<< ${probe_output})  
  width=$(jq -r '.streams[0].width' <<< ${probe_output})  

  # skip if codec not h264  
  if [[ "$codec" != "h264" && "$codec" != "libx264" ]]; then  
    log INFO "  video streams found are not h264/libx264"  
    continue  
  fi  

  # Only process 4k files  
  if [[ ${width} -gt ${height} ]]; then  
    resolution=${height}  
  else  
    resolution=${width}  
  fi  
  if [[ ${resolution} -lt 2160 ]]; then  
    log INFO "  ok (video resolution is under 4k)"  
    continue  
  fi  

  # normalize level:  
  # ffprobe may return "51" or "5.1"; convert "5.1" -> 51 (integer) for comparison.  
  if [[ -z "$level_raw" ]]; then  
    level_int=0  
  elif [[ "$level_raw" == *.* ]]; then  
    # multiply by 10 and floor  
    level_int=$(awk -v l="$level_raw" 'BEGIN{printf("%d", l*10)}')  
  else  
    # ensure base-10 integer  
    level_int=$((10#$level_raw)) 2>/dev/null || level_int=0  
  fi  

  if (( level_int > 51 )); then  
    # look for othe files of same name root, but with different bit rate  
    froot=$(sed -r "s,(.*)\[[0-9\.]+ Mbps\]\.${f##*.},\1," <<< $f)  
    fcount=$(/bin/ls -1 "${froot}"*.${f##*.} | wc -l)  
    if [[ 2 -le $fcount ]]; then  
      log INFO "  Found other files with same name root and different bitrate, skipping"  
      continue  
    fi  

    need_convert=1  
    # prefer first non-empty profile we find  
    if [[ -z "$picked_profile" && -n "$profile" ]]; then  
      picked_profile="$profile"  
    fi  
  fi  

  if (( need_convert )); then  
    log INFO "  will convert (found h264 level > 5.1). profile='${picked_profile:-(none)}'"  

    # create a unique temporary file in the same directory  
    out="${f%.*}.level51"  
    tmpdir="$(dirname "${out}.${f##*.}")"  
    tmptmpl="$(basename "${out}").tmp.XXXXXX"  
    tmpfile="$(mktemp "$tmpdir/$tmptmpl")" || { log WARN "  failed to create temp file"; continue; }  
    rm -f -- "$tmpfile"  # remove placeholder so ffmpeg can create it with correct container ext  
    tmp="${tmpfile}.${f##*.}"  

    if [[ -n "$picked_profile" ]]; then  
      ffargs=( -i "$f" -v error -map 0 -c:a copy -c:s copy -c:v libx264 -profile:v "$picked_profile" -level 5.1 -preset slow -crf 18 -y "$tmp" )  
    else  
      ffargs=( -i "$f" -v error -map 0 -c:a copy -c:s copy -c:v libx264 -level 5.1 -preset slow -crf 18 -y "$tmp" )  
    fi  

    if (( DRY_RUN )); then  
      log INFO "  DRY RUN -- Running: ffmpeg ${ffargs[*]}"  
      continue  
    fi  

    log INFO "  Running: ffmpeg ${ffargs[*]}"  
    if ffmpeg "${ffargs[@]}"; then  
      bit_rate_reduced=$(ffprobe -v error -of json -show_format -show_entries format=bit_rate "$tmp" | jq -r '.format.bit_rate')  
      bit_rate_reduced_mbps=$(awk -v br=${bit_rate_reduced} 'BEGIN{printf("%2.2f", br/1000000)}')  
      mv -v -- "$f" "${f%.*}.[${bit_rate_mbps} Mbps].${f##*.}"  
      mv -v -- "$tmp" "${f%.*}.[${bit_rate_reduced_mbps} Mbps].${f##*.}"  
      log INFO "  converted -> $f"  
    else  
      log WARN "  ffmpeg failed for $f" >&2  
      rm -f -- "$tmp"  
    fi  
  else  
    log INFO "  ok (no h264 stream with level > 5.1)"  
  fi  
} done <<< $(find "${START_DIR}" -type f -iname '*.mp4' -o -iname '*.mkv') 

log INFO "\n\n---------------- Finished run ----------------\n\n"  

  • Badabinski@kbin.earth
    link
    fedilink
    arrow-up
    8
    ·
    22 hours ago

    For future reference, you should remove set -euo pipefail and then add manual error handling to all commands which could fail. set -euo pipefail is one of the biggest, most disgusting foot guns in bash (a shit language cursed with many horrible foot guns) and does nothing but mask your problems. If every command has a unique error message, then you can much more easily track down your problems.

    To make things more ergonomic, remove the foot gun and add a die function that accepts an error message and optional exit code and calls it like this:

    echoerr() { echo "$@" 1>&2; }
    
    die() {
      message="$1"; shift
      exit_code="${1:-1}"
      echoerr "$message"
      exit "$exit_code"
    }
    
    fallible_command || die 'shit is so fucked yo'
    

    source: bash was my first language and I’ve been dealing with its bullshit personally and professionally for over 13 years now.

  • TwilightKiddy
    link
    fedilink
    English
    arrow-up
    7
    ·
    24 hours ago

    Is it the infamous “run ffmpeg in a while loop and have it slurp your stdin” case? Try adding -nostdin to your ffmpeg call.

    • baronvonj@piefed.socialOP
      link
      fedilink
      English
      arrow-up
      5
      ·
      edit-2
      23 hours ago

      ah, I wasn’t aware of that option, hopefully that’s it.

      update - that was it, thanks!

  • Ephera@lemmy.ml
    link
    fedilink
    arrow-up
    2
    arrow-down
    1
    ·
    1 day ago

    Not a Bash pro, but those braces look weird…?

    while read f; do {
    
    } done
    

    The internet tells me, there’s an undocumented feature that allows braces in loops, but then without do / done

    • baronvonj@piefed.socialOP
      link
      fedilink
      English
      arrow-up
      2
      ·
      24 hours ago

      I’ve always written my while/for loops with braces like this.

      $ egrep '(while|done)' ~/.local/bin/*.sh
      /home/baronvonj/.local/bin/disable_subtitle_tracks.sh:while read file; do {
      /home/baronvonj/.local/bin/disable_subtitle_tracks.sh:} done < ${file_list}
      /home/baronvonj/.local/bin/extract_subtitles.sh:while read fpath; do {
      /home/baronvonj/.local/bin/extract_subtitles.sh:} done <<<$(find "$1" -name "*.mp4" -o -name "*.mkv")
      /home/baronvonj/.local/bin/find_4k_level52_h264.sh:while read file; do {
      /home/baronvonj/.local/bin/find_4k_level52_h264.sh:} done <<< $(find $1 -name "*.mp4" -o -name "*.mkv")
      /home/baronvonj/.local/bin/find_4k_only.sh:while read line; do {
      /home/baronvonj/.local/bin/find_4k_only.sh:} done <<< $(find "${mdir}" -regextype posix-extended -regex '.*\[4k\]\.(mp4|mkv)')
      /home/baronvonj/.local/bin/reduce_level.sh:while true; do
      /home/baronvonj/.local/bin/reduce_level.sh:done
      /home/baronvonj/.local/bin/reduce_level.sh:while read f; do {
      /home/baronvonj/.local/bin/reduce_level.sh:} done <<< $(find "${START_DIR}" -type f -iname '*.mp4' -o -iname '*.mkv') 
      /home/baronvonj/.local/bin/remove_languages.sh:while read file; do {
      /home/baronvonj/.local/bin/remove_languages.sh:} done < ${file_list}
      /home/baronvonj/.local/bin/reorg_v2_v3.sh:while read src; do {
      /home/baronvonj/.local/bin/reorg_v2_v3.sh:} done <<< $(find "${indir}" -name "*.mp4" -o -name "*.mkv" -o -name "*.nfo")
      /home/baronvonj/.local/bin/set_quality.sh:while read fpath; do {
      /home/baronvonj/.local/bin/set_quality.sh:} done <<<$(find "$1" -name "*.mp4" -o -name "*.mkv")```
      • TwilightKiddy
        link
        fedilink
        English
        arrow-up
        1
        ·
        24 hours ago

        It doesn’t actually matter, but the syntax indeed does not include {}. You are just creating an extra grouping for no reason.

        • baronvonj@piefed.socialOP
          link
          fedilink
          English
          arrow-up
          2
          ·
          23 hours ago

          extra grouping for no reason.

          I just find it more readable/foldable with the braces. I learned C/C++/Java before bash.

          • TwilightKiddy
            link
            fedilink
            English
            arrow-up
            4
            ·
            23 hours ago

            No harm in it apart from people looking weird at you.

            A bit more important: wrap your parameter expansions in double quotation marks. E.g. ${probe_output}"${probe_output}" Otherwise bash will do globbing and splitting on it and you most likely don’t want that. Often times it’s not required, but it’s a good habit to have just in case.

            Another one is passing -r to read so that it won’t feast on your backslashes.

  • bombadil@feddit.org
    link
    fedilink
    arrow-up
    1
    ·
    1 day ago

    no time to read your whole code rn so not sure if its the same cause, but i recently had a similar issue with a while read loop only running once. it was caused by something in the loop body reading from stdin and eating all the subsequent file names before the next read could get to it

    • baronvonj@piefed.socialOP
      link
      fedilink
      English
      arrow-up
      1
      ·
      1 day ago

      It’s not just doing one loop iteration and then completing. It will correctly loop through all the files as long as none of them need to be converted. Once it finds one that needs to be converted, it does the conversion and then exits. I was seeing ffmpeg dropping into command mode, which suggested it was detecting stdin, that was part of why I wanted to put it in a backgrounded non-tty container. I’m not seeing that output anymore, but still seeing it exit the loop. I’m guessing it’s something in the if ffmpeg "${ffargs[@]}"; then I’ll try splitting those up.