0

The following script is meant to trim all media files in the current working directory.

#!/usr/bin/bash

trimmer() {
  of=$(echo "${if}"|sed -E "s/(\.)([avimp4kvweb]{3,3}$)/\1trimmed\.\2/")
  ffmpeg -hide_banner -loglevel warning -ss "${b}" -to "${ddecreased}" -i "${if}" -c copy "${of}"
  echo "Success. Exiting .."
}

get_ddecreased() {
  duration="$(ffprobe -v quiet -show_entries format=duration -hide_banner "${if}"|grep duration|sed -E s/duration=\([0-9]*\)\..*$/\\1/)"
  echo ${duration} 
  ddecreased="$(echo "${duration} - ${trimming}"|bc -l)"
  echo ${ddecreased} 
}

rm_source() {
  echo -e "Remove ${if}?[Y/n]?"
  read ch
  if [[ "${ch}" == 'y' ]]; then
    rm "${if}"
  fi 
}


echo "How much of the beginning would you like to trim?"
read b
echo "How much of the end would you like to trim?"
read trimming

ls *.avi *.mkv *.mp4 *.vob >list_of_files

echo "Prompt before removing the source[Y/n]?"
read ch
while IFS="" read -r if || [[ -n "${if}" ]]; do
  if [[ "${ch}" == 'y' ]]; then
    get_ddecreased  && trimmer && rm_source
  elif [[ "${ch}" == 'n' ]]; then
    get_ddecreased && trimmer && rm "${if}"
  fi
  echo $if
done <list_of_files

echo -e "Removing list_of_files."
rm list_of_files 

If the user selected y when asked Prompt before removing the source[Y/n] and trimmer has finished trimming the first file rm_source is meant to prompt the user and wait for their input before removing the source file. This does not work as the script does not wait for the input and proceeds straight away to echo -e "Removing list_of_files." much like there was no while loop at all. Neither does the while loop get executed when the user selected n when asked Prompt before removing the source[Y/n] - the script proceeds straight away to echo -e "Removing list_of_files." instead of iterating through all the files listed in list_of_files. Why so? Yet when I comment out all these lines

if [[ "${ch}" == 'y' ]]; then
    get_ddecreased  && trimmer && rm_source
  elif [[ "${ch}" == 'n' ]]; then
    get_ddecreased && trimmer && rm "${if}"
  fi

within the while loop all the lines of list_of_files get printed to the screen.

What is wrong with my code?

2
  • 4
    Think about where input is coming from for the read call in your function. Hint: Standard input is no longer connected to a terminal there.
    – Kusalananda
    Aug 21, 2022 at 13:41
  • 1
    Sidebar - using ls in scripting is a bad idea; just put that in a for loop or use find. Aug 21, 2022 at 14:02

1 Answer 1

3

Your code is essentially doing the following:

foo () {
    read variable
}

while read something; do
    foo
done <input-file

The intention is to have the read in foo read something from the terminal, however, it is being called in a context where the standard input stream is redirected from some file.

This means that the read in foo will read from the input stream coming from the input file, not from the terminal at all.

You may circumvent this by making the loop read from a another file descriptor than standard input:

foo () {
    read variable
}

while read something <&3; do
    foo
done 3<input-file

Here, the read in the loop reads from file descriptor 3, which is being connected to the input file after the done keyword. This leaves the read in the foo function free to use the original standard input stream.

In the bash shell, rather than using a hard-coded value for the extra filedescriptor, you can have the shell allocate the descriptor in a shell variable:

foo () {
    read variable
}

while read something <&"$fd"; do
    foo
done {fd}<input-file

This would likely set $fd to an integer like 10 or higher. The exact value is unimportant.


In your current code in the question, you may also fix your issue by avoiding creating and reading from the list of files, and instead use the file globs directly:

for filename in *.avi *.mkv *.mp4 *.vob; do
    if [ ! -e "$filename" ]; then
        # skip non-existing names
        continue
    fi

    # use "$filename" here
    # ... and call your rm_source function
done

This avoids redirections all-together. This also allows your code to handle the odd file with newline characters in its name.

The if statement in the loop, which tests for the existence of the named file, is necessary as the shell will, by default, retain the globbing pattern if there are no matching names for that pattern. You may get rid of the if statement in the bash shell by setting the nullglob shell option using shopt -s nullglob. Setting this option would make the bash shell remove non-matching globs completely.

Note too that this is not the same as in your code if any name matching the globbing patterns is a directory. If you have a directory called e.g. mydir.mp3, then ls would list the contents of that directory. Also, if a filename matching the patterns starts with a dash, the code using ls would likely mistake that name for a set of options.

8
  • Is while IFS="" read l <&7; equivalent to while IFS="" read -u 7 -r l;? I've been using the latter in the past but it occasionally lead to unpredictable results (sometimes there was something more in this descriptor than what I intended to redirect) and so I stopped using today. But the cure turned out to be event was that the original issue...
    – John Smith
    Aug 21, 2022 at 16:22
  • 1
    @JohnSmith The two commands in your comment are equivalent in the bash shell, yes.
    – Kusalananda
    Aug 21, 2022 at 17:11
  • 1
    @JohnSmith Since you have modified the code in the question in such a way that the file list file is only used for the loop, you may get rid of it completely, which also solves your issue. See updated answer.
    – Kusalananda
    Aug 21, 2022 at 17:16
  • 2
    @JohnSmith, those aren't the same as the second one has the -r option to read (keep backslashes as-is), and the first one doesn't. Usually you want read -r. As for -u 7 vs. <&7, -u` might be slightly better in terms of performance, since there's less file descriptor juggling involved. Basically, with -u 7 the shell just reads directly from fd 7, but with <&7 it needs to duplicate fd 7 to fd 0 and then read from there (saving the original fd 0 for the while).
    – ilkkachu
    Aug 21, 2022 at 19:15
  • 1
    for filename in *.avi *.mkv *.mp4 *.vob has the caveat that any unmatching globs would appear as-is by default, so that may need checking in the loop or just using shopt -s nullglob. (ls would print error messages for them but nothing to stdout)
    – ilkkachu
    Aug 21, 2022 at 19:17

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .