lisamelton / other_video_transcoding

Other tools to transcode videos.
MIT License
540 stars 24 forks source link

Add `--output` option. #182

Closed Goorzhel closed 1 year ago

Goorzhel commented 1 year ago

Code entirely borrowed from transcode-video (resolve_output, opts.on '--output ARG', help text). For consistency's sake, I skipped the short option (-o). Also, I appreciated that the change to use resolve_output was a one-liner.

There are two minor COMBAK notes for your feedback (lines 480 and 1190 in 241b41e).

Dockerfile I'm on NixOS and way too lazy to set up the dependencies at the OS level. I might send this file as a separate PR, although I don't have a Docker Hub account with which to get the image to the masses. ``` FROM ruby:3-slim RUN apt-get update \ && apt-get install -y ffmpeg mkvtoolnix mpv \ && useradd -Nmu 1000 app USER 1000 COPY --chown=1000 . /app WORKDIR /home/app ENTRYPOINT ["/app/bin/other-transcode"] ```
Test script ``` #!/usr/bin/env bash set -x mkdir outdir transcode() { sudo docker run --rm -it -v "$PWD:$PWD" -w "$PWD" other-transcode "$@" } transcode whosonfirst.mkv &> output_to_pwd transcode whosonfirst.mkv --output outdir &> output_to_dir transcode whosonfirst.mkv --output outfile.mkv &> output_to_outfile exa -lR ```
Results ``` ag@server /dev/shm ❯ exa -l .rwxr-xr-x 311 ag 19 Mar 17:11 test .rw-r--r-- 39M ag 19 Mar 17:12 whosonfirst.mkv ag@server /dev/shm ❯ ./test + mkdir outdir + transcode whosonfirst.mkv + transcode whosonfirst.mkv --output outdir + transcode whosonfirst.mkv --output outfile.mkv + exa -lR drwxr-xr-x - ag 19 Mar 17:14 outdir .rw-r--r-- 98M ag 19 Mar 17:15 outfile.mkv .rw-r--r-- 6.9k ag 19 Mar 17:15 outfile.mkv.log .rw-r--r-- 14k ag 19 Mar 17:14 output_to_dir .rw-r--r-- 14k ag 19 Mar 17:15 output_to_outfile .rw-r--r-- 364 ag 19 Mar 17:13 output_to_pwd .rwxr-xr-x 311 ag 19 Mar 17:12 test .rw-r--r-- 39M ag 19 Mar 17:12 whosonfirst.mkv ./outdir: .rw-r--r-- 98M ag 19 Mar 17:14 whosonfirst.mkv .rw-r--r-- 6.9k ag 19 Mar 17:14 whosonfirst.mkv.log ``` (The size difference? I suspect the VP9 original has a much lower bitrate than the 1.5 MB/s the AVC output was written in, but `mediainfo` won't tell me what it is. I needed a very small video to test with anyway.)
Relevant log lines You can see at the end of the ffmpeg commands that the output path resolved correctly. ``` ag@server /dev/shm ❯ tail -1 output_to_pwd /app/bin/other-transcode: output file exists: whosonfirst.mkv ag@server /dev/shm ❯ grep ^ffmpeg output_to_dir ffmpeg -loglevel error -stats -i whosonfirst.mkv -map 0:0 -c:v libx264 -b:v 1500k -maxrate:v 10000k -bufsize:v 10000k -mbtree:v 0 -profile:v high -color_primaries:v bt709 -color_trc:v bt709 -colorspace:v bt709 -metadata:s:v title\= -disposition:v default -map 0:1 -c:a:0 aac -metadata:s:a:0 title\= -disposition:a:0 default -sn -metadata:g title\= -default_mode passthrough /dev/shm/outdir/whosonfirst.mkv ag@server /dev/shm ❯ grep ^ffmpeg output_to_outfile ffmpeg -loglevel error -stats -i whosonfirst.mkv -map 0:0 -c:v libx264 -b:v 1500k -maxrate:v 10000k -bufsize:v 10000k -mbtree:v 0 -profile:v high -color_primaries:v bt709 -color_trc:v bt709 -colorspace:v bt709 -metadata:s:v title\= -disposition:v default -map 0:1 -c:a:0 aac -metadata:s:a:0 title\= -disposition:a:0 default -sn -metadata:g title\= -default_mode passthrough /dev/shm/outfile.mkv ```
Goorzhel commented 1 year ago

Ah, joke's on me for not reading the other PRs:

And I will definitely not take a patch which adds an --output option. Sorry, but it's by design that other-transcode does not have that. It's simply unnecessary.

I have my uses for it, but I'll keep it in my local copy henceforth.

apenngrace commented 3 months ago

@lisamelton, I hope you might reconsider including an --output or -o option. I'm running into name collision issues. For example, if I start with a file A.mkv, I am unable to get an mkv output. I have to select the --mp4 option in order to get output.

Additionally, because the transcode-video project included the -o option, I was able to save files into other directories. That flexibility was nice to have.

ttyS0 commented 3 months ago

Alternatively ...

mkdir source
mv A.mkv source/
other-transcode source/A.mkv
apenngrace commented 3 months ago

@ttyS0, I appreciate the work-around tip. Respectfully, I still hope that @lisamelton will consider the value of accepting the pull request to add the --output feature.

lisamelton commented 3 months ago

@apenngrace The name collision is an intentional feature. It's to prevent you from transcoding your output into the same working directory as your source. Seriously. Too many users have inadvertently deleted their source files by using the same working directory for both source and output.

Follow @ttyS0 's fine advice or simply cd to a directory where you want your output and specify your input by its full or qualified path.

loshlee commented 3 months ago

Lisa has intentionally designed this into the solution. It's been discussed many times. Because the workaround (changing the present working directory) is so simple, there's nothing to be gained by repeating the request.

lisamelton commented 3 months ago

@loshlee Thanks! You totally get it. 👍

apenngrace commented 3 months ago

@lisamelton, right, I understand that we would not want to overwrite source file. But your code already checks if the output file already exists. This pull request does not introduce that problem.

I just tried this pull request out. I used the --output option and attempted to overwrite the source file. It stopped me.

@loshlee and @lisamelton, anyways, I understand the work-around. It was a nice option to have when it was available on your other project. It was a feature I liked, that's all. I will go ahead and follow your suggestions.

lisamelton commented 3 months ago

@apenngrace I think you misunderstand. Of course I wouldn't allow other-transcode to overwrite an existing file. The limitation/feature is there to make it inconvenient to use the same working directory for both input and output. Not because other-transcode will delete an existing file, but because the user might do that with some other action.

Many users of my original transcoding script, myself included, learned that lesson the hard way.