Skip to content

Add --output option. #182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add --output option. #182

wants to merge 1 commit into from

Conversation

Goorzhel
Copy link

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
Copy link
Author

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.

@Goorzhel Goorzhel closed this Mar 20, 2023
@apenngrace
Copy link

@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.

@skj-dev
Copy link

skj-dev commented Mar 10, 2024

Alternatively ...

mkdir source
mv A.mkv source/
other-transcode source/A.mkv

@apenngrace
Copy link

@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
Copy link
Owner

@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
Copy link

loshlee commented Mar 10, 2024

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
Copy link
Owner

@loshlee Thanks! You totally get it. 👍

@apenngrace
Copy link

@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
Copy link
Owner

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants