lisamelton / video_transcoding

Tools to transcode, inspect and convert videos.
MIT License
2.39k stars 160 forks source link

batch.sh --burn-srt not picking up .srt #236

Closed davidjoelhall closed 5 years ago

davidjoelhall commented 5 years ago

I attempted to adapt the batch.sh file removing --crop and adding --burn-srt . The transcode completes, but no .srt file is loaded so --burn-srt doesn't complete.

The batch file that I'm using is batch.txt attached, just renamed from batch.sh for uploading purposes.

batch.txt queue.txt S01E05.mp4.log

lisamelton commented 5 years ago

@davidjoelhall It appears that your batch file never passed the --burn-srt argument to transcode-video. What happens when you don't use the batch file but type an explicit command instead?

davidjoelhall commented 5 years ago

When I do transcode-video --mp4 --burn-srt /path/to/subtitle.srt /path/to/movie.mkv it transcodes and burns in the subtitle properly.

lisamelton commented 5 years ago

@davidjoelhall Good! :) OK, that means the bug is in your batch.sh file. Without having your filesystem environment in front of me, my guess is that you have a failure finding the subtitle file.

davidjoelhall commented 5 years ago

That's what I figured. Here's what I have and what I do...

Directory where I have the files, /username/work

Within /work, I have:

Within queue.txt I drag the file(s) to convert into the plain text file to get the proper paths, generally it's just /username/work/S01E05.mkv.

Within /subtitles, I have:

When it comes time to run batch.sh I cd to /username/work and run ./batch.sh.

lisamelton commented 5 years ago

@davidjoelhall Why don't you add some debugging code to your batch file, like this:

#!/usr/bin/env bash

readonly work="$(cd "$(dirname "$0")" && pwd)"
readonly queue="$work/queue.txt"
readonly subtitles="$work/subtitles"

input="$(sed -n 1p "$queue")"

while [ "$input" ]; do
    title_name="$(basename "$input" | sed 's/\.[^.]*$//')"
    echo "title_name =$title_name"
    subtitle_file="$subtitles/${title_name}.srt"
    echo "subtitle_file =$subtitle_file"

    if [ -f "$subtitle_file" ]; then
        subtitle_option="--burn-srt $(cat "$subtitle_file")"
    else
        subtitle_option=''
    fi

    echo "subtitle_option =$subtitle_option"

    sed -i '' 1d "$queue" || exit 1

    transcode-video --mp4 "$input"

    input="$(sed -n 1p "$queue")"
done

That could localize the problem.

davidjoelhall commented 5 years ago

Oddly enough, the log doesn't have any indication of a file being passed through to transcode-video; however, when I scrolled up in the results from Terminal, I could see the text of .srt file echoed throughout the beginning: subtitle text.txt

I've attached the log to see if you can find anything worth while. S01E05.mp4.log

Note: the attached file, subtitle text, is for the next transcode as I didn't think to save the previous Terminal log as I thought this information was going to be part of S01E05.mp4.log.

lisamelton commented 5 years ago

@davidjoelhall OK, now I see the bug in your batch.sh file. :) My apologies for not seeing it earlier but I was in the process of releasing a new version of this Gem.

Just change this line:

        subtitle_option="--burn-srt $(cat "$subtitle_file")"

to this:

        subtitle_option="--burn-srt $subtitle_file"

All the --burn-srt option needs is the filename, not the actual contents of the file.

However, keep in mind that quoting the the option and filename together to set the variable is problematic. I won't work if the filename every has a space or non-alphanumeric character in it.

davidjoelhall commented 5 years ago

@donmelton sweet! I'll change this now and test it.

Do you have a recommendation for how to do this for many files? I'd imagine that if I have 20 files, all with the same base name, this will still work?

lisamelton commented 5 years ago

@davidjoelhall To handle spaces in filenames (and actually pass the subtitle options to transcode-video), you could rewrite your batch.sh file like this:

#!/usr/bin/env bash

readonly work="$(cd "$(dirname "$0")" && pwd)"
readonly queue="$work/queue.txt"
readonly subtitles="$work/subtitles"

input="$(sed -n 1p "$queue")"

while [ "$input" ]; do
    title_name="$(basename "$input" | sed 's/\.[^.]*$//')"
    subtitle_file="$subtitles/${title_name}.srt"

    if [ -f "$subtitle_file" ]; then
        subtitle_options=(--burn-srt "$subtitle_file")
    else
        subtitle_options=()
    fi

    sed -i '' 1d "$queue" || exit 1

    transcode-video --mp4 "${subtitle_options[@]}" "$input"

    input="$(sed -n 1p "$queue")"
done

I hope that helps.

lisamelton commented 5 years ago

@davidjoelhall I'm going to close this now because I assume that we've solved the problem. But if that's not the case and you run into a problem again, feel free to reopen this or just comment again here.

davidjoelhall commented 5 years ago

My apologies for the lack of response here... The most recent transcode batch didn't work and I was turning over the idea of just leaving this alone. I figured if someone else comes across this though, it could be helpful.

After doing another transcode, I figured I'd rerun and open this to see if I missed something.

This time though, I saved the output from Terminal: S01E06 - terminal log.txt

Here's the transcode-video log: S01E06.mp4.log

Here's the batch.sh file saved as batch.txt for upload purposes: batch.txt

If I'm barking up the wrong tree or doing something silly, please let me know. I transcoded the same file "manually" using transcode-video --mp4 --burn-srt S01E06.srt S01E06.mkv and confirmed the output had burned in subtitles appropriately.

lisamelton commented 5 years ago

@davidjoelhall Reopening this to make it easier.

That batch.sh file of yours will never work. This line:

    transcode-video --mp4 "$input"

... doesn't even pass the --burn-srt argument anywhere.

That's why it's failing.

davidjoelhall commented 5 years ago

Well, as I said... if I'm doing something silly.

Am I correct thinking that changing that line to:

transcode-video --mp4 --burn-srt "$subtitle_file" "$input"

would resolve the issue? Then that would work for batch files as well, if I'm reading batch.sh correctly.

edit: tried, failed. Re-thinking this...

davidjoelhall commented 5 years ago

I kept seeing /usr/local/bin/transcode-video: invalid option: --burn-srt /Users/dh/work/subtitles/S01E10.srt with no video file being passed through... so I tried doing transcode-video --mp4 --burn-srt /Users/dh/work/subtitles/S01E10.srt S01E10.mkv (to check if the full file path worked) which worked.

Now working out what's wrong with the "$input".

FWIW, Shortcuts on iOS has spoiled me.

lisamelton commented 5 years ago

@davidjoelhall No worries. Scripting is hard. And you're learning the right way. Via the old lather, rinse, repeat process.

Try a script like this:

#!/usr/bin/env bash

readonly work="$(cd "$(dirname "$0")" && pwd)"
readonly queue="$work/queue.txt"
readonly subtitles="$work/subtitles"

input="$(sed -n 1p "$queue")"

while [ "$input" ]; do
    title_name="$(basename "$input" | sed 's/\.[^.]*$//')"
    subtitle_file="$subtitles/${title_name}.srt"

    if [ -f "$subtitle_file" ]; then
        subtitle_options=(--burn-srt "$subtitle_file")
    else
        subtitle_options=()
    fi

    sed -i '' 1d "$queue" || exit 1

    transcode-video --mp4 "${subtitle_options[@]}" "$input"

    input="$(sed -n 1p "$queue")"
done

Notice how I'm putting both the --burn-srt option and the "$subtitle_file" value as separate elements in an array. And when that array is empty, the transcode-video line will still work.

I hope that helps.

lisamelton commented 5 years ago

@davidjoelhall So, did that new batch.sh work?

davidjoelhall commented 5 years ago

@donmelton best I can tell, you're a damn wizard and I thank you. Not only for transcode-video but for the help with getting batch.sh re-worked for subtitles.

Thank you. Close up this request.

lisamelton commented 5 years ago

@davidjoelhall Thanks and you are very welcome, sir! I'm glad it all worked out for you.