-o isn't passing through file path
arikalish opened this issue · 32 comments
Starting with this output from transcode-video --scan against a DVD folder of the most recent Hitchhiker's Guide (not a .mkv file):
+ title 12:
+ Main Feature
+ vts 2, ttn 1, cells 0->25 (2584340 blocks)
+ duration: 01:48:40
+ size: 720x480, pixel aspect: 32/27, display aspect: 1.78, 29.970 fps
+ chapters:
+ 1: cells 0->0, 107651 blocks, duration 00:03:52
+ 2: cells 1->1, 76584 blocks, duration 00:02:54
+ 3: cells 2->2, 155302 blocks, duration 00:06:10
+ 4: cells 3->3, 44048 blocks, duration 00:01:36
+ 5: cells 4->4, 107001 blocks, duration 00:04:01
+ 6: cells 5->5, 165070 blocks, duration 00:07:06
+ 7: cells 6->6, 111907 blocks, duration 00:04:35
+ 8: cells 7->7, 90249 blocks, duration 00:04:16
+ 9: cells 8->8, 121317 blocks, duration 00:05:33
+ 10: cells 9->9, 100528 blocks, duration 00:03:49
+ 11: cells 10->10, 104069 blocks, duration 00:04:46
+ 12: cells 11->11, 81954 blocks, duration 00:03:52
+ 13: cells 12->12, 97624 blocks, duration 00:04:01
+ 14: cells 13->13, 110054 blocks, duration 00:04:29
+ 15: cells 14->14, 113364 blocks, duration 00:05:12
+ 16: cells 15->16, 93784 blocks, duration 00:04:03
+ 17: cells 17->17, 85576 blocks, duration 00:03:43
+ 18: cells 18->18, 113054 blocks, duration 00:04:36
+ 19: cells 19->19, 115980 blocks, duration 00:04:27
+ 20: cells 20->20, 79622 blocks, duration 00:02:55
+ 21: cells 21->21, 110249 blocks, duration 00:04:45
+ 22: cells 22->22, 128655 blocks, duration 00:05:03
+ 23: cells 23->23, 154794 blocks, duration 00:06:04
+ 24: cells 24->24, 115717 blocks, duration 00:06:53
+ 25: cells 25->25, 187 blocks, duration 00:00:01
+ audio tracks:
+ 1, English (AC3) (5.1 ch) (iso639-2: eng), 48000Hz, 448000bps
+ 2, English (DTS) (5.1 ch) (iso639-2: eng), 48000Hz, 768000bps
+ 3, Francais (AC3) (2.0 ch) (Dolby Surround) (iso639-2: fra), 48000Hz, 192000bps
+ 4, Espanol (AC3) (2.0 ch) (Dolby Surround) (iso639-2: spa), 48000Hz, 192000bps
+ 5, English (AC3) (Director's Commentary 1) (1.0 ch) (iso639-2: eng), 48000Hz, 96000bps
+ 6, English (AC3) (Director's Commentary 2) (1.0 ch) (iso639-2: eng), 48000Hz, 96000bps
+ subtitle tracks:
+ 1, English (Closed Caption) (iso639-2: eng) (Bitmap)(VOBSUB)
+ 2, Francais (iso639-2: fra) (Bitmap)(VOBSUB)
+ 3, Espanol (iso639-2: spa) (Bitmap)(VOBSUB)
+ 4, Francais (iso639-2: fra) (Bitmap)(VOBSUB)
+ 5, Espanol (iso639-2: spa) (Bitmap)(VOBSUB)
+ 6, Closed Captions (iso639-2: eng) (Text)(CC)
I tried this command:
transcode-video --title 12 -o /Volumes/Videos/Converted/HGG0NND1.m4v --m4v --crop detect --add-audio 1 --copy-audio 2 --burn-subtitle scan --add-subtitle 1 --add-subtitle 6 --dry-run /Volumes/HGG0NND1/
And received this output:
HandBrakeCLI --input=/Volumes/HGG0NND1/ --output=/Users/ari/.rvm/rubies/HGG0NND1.m4v --markers --encoder=x264 --quality=16 --title=12 --crop=60:64:0:0 --strict-anamorphic --rate=23.976 --audio=1,1 --aencoder=ca_aac,copy --audio-fallback=ac3 --subtitle=scan,1,6 --subtitle-burned --encopts=vbv-maxrate=2000:vbv-bufsize=1000:crf-max=25
the --audio=1,1 --aencoder=ca_aac,copy section seems wrong. I would have expected something more like --audio=1,1,2 --aencoder=ca_aac,ac3,copy:dts
I'm also noticing that the output file name isn't getting passed through correctly to HandbrakeCLI. Based on
-o /Volumes/Videos/Converted/HGG0NND1.m4v
shouldn't HandbrakeCLI get
--output=/Volumes/Videos/Converted/HGG0NND1.m4v
Per our Twitter conversation, adding --add-audio 2 before the --copy-audio 2 fixed the audio issue. Still seeing the -o not getting passed through:
transcode-video --title 12 -o /Volumes/Videos/Converted/HitchHiker.m4v --m4v --crop detect --add-audio 2 --copy-audio 2 --burn-subtitle scan --add-subtitle 1 --add-subtitle 6 --dry-run /Volumes/HGG0NND1/
HandBrakeCLI --input=/Volumes/HGG0NND1/ --output=/Users/ari/.rvm/rubies/HitchHiker.m4v --markers --encoder=x264 --quality=16 --title=12 --crop=60:64:0:0 --strict-anamorphic --rate=23.976 --audio=1,1,2 --aencoder=ca_aac,copy,copy --audio-fallback=ac3 --subtitle=scan,1,6 --subtitle-burned --encopts=vbv-maxrate=2000:vbv-bufsize=1000:crf-max=25
@arikalish Thanks for filing this!
OK, as we discussed on Twitter, the correct command (for now) should be:
transcode-video --add-audio 2 --copy-audio 2 "/path/to/Movie/"
Sorry that it works that way since it's a bit redundant. I'll either document it better or make it so the --add-audio option is implicit when you do --copy-audio. Not sure which approach to take yet since I'm low on sleep at the moment. :)
But I'll leave this open until we resolve the other issue. That's very possibly a real bug. Looking at it now...
@arikalish I can't reproduce the "output" bug no matter how hard I try. I suspect this may have something to do with the version of Ruby you're using. Perhaps something changed in the standard library?
I was running 2.3 from rvm. Backed down to 2.2.2p95 from Homebrew per your suggestion. Closed and reopened terminal window and still getting same output (pwd has moved to my home folder):
~ ari$ transcode-video --title 12 -o /Volumes/Videos/Converted/HitchHiker.m4v --m4v --crop detect --add-audio 2 --copy-audio 2 --burn-subtitle scan --add-subtitle 1 --add-subtitle 6 --dry-run /Volumes/HGG0NND1/
HandBrakeCLI --input=/Volumes/HGG0NND1/ --output=/Users/ari/HitchHiker.m4v --markers --encoder=x264 --quality=16 --title=12 --crop=60:64:0:0 --strict-anamorphic --rate=23.976 --audio=1,1,2 --aencoder=ca_aac,copy,copy --audio-fallback=ac3 --subtitle=scan,1,6 --subtitle-burned --encopts=vbv-maxrate=2000:vbv-bufsize=1000:crf-max=25
If it matters (can't see why it should), /Volumes/Videos is a SMB share.
Just tried it with an SMB share and it works fine, so that's not it.
Stupid question: does the "Converted" folder in "/Volumes/Videos" actually exist?
Indeed. It's the output folder I've been using for ages for Handbrake GUI. Just cd'd to it and started an encode. Seems to be happy enough.
OK, but let's leave this issue open until the real problem is resolved. I'll continue to test. Thanks for all your help!
Absolutely. Let me know if there's anything I can try to help debug.
Thanks for the re-title!
@arikalish For the first problem, I've decided to clarify in the "README" that --copy-audio, like --audio-width, doesn't automatically "add" the selected track and that you have to also explicitly call --add-audio.
I know that sounds inconvenient but it's all part of my master plan to convince people that they should never copy audio tracks. :)
That works for me!
I put all the DVDs into the basement and only pull them out if necessary. We've got a decent audio setup on the TV hooked up to a Mac Mini, so might as well use it if we can. We've got a bunch of movies from iTunes so I try to keep everything consolidated instead of having to dig around for disks or other apps. Maybe someday we'll move to Plex.
Our iPads have enough space that I'm not too worried about the extra audio.
OK, I've just checked in a clarification in the "README". Let me know what you think.
I'll keep this open while we try to figure out the other problem.
Makes sense to me. Might be good to put a note in the --help output as well
OK, I'll see if I can squeeze that in there.
Maybe something like "requires matching --add-audio for all non-default tracks" ?
Not bad. Not bad at all.
You know, I should make the same --help change for the --copy-audio-name option, too, since the semantics are the same.
Seems reasonable!
@arikalish I've been trying to repro the "-o isn't passing through file path" problem for a half hour now and still no luck. Is this still happening to you with the latest 0.1.4 version of the Gem?
Honestly haven't looked at it since. I'm on a business trip for the next week but plan on digging through the Ruby to try and suss it out on my machine when I return.
@arikalish I just updated the code to clarify in --help output that transcode-video audio copy policies
only apply to main and explicitly added audio tracks. I haven't released a Gem with this change yet but that will probably happen later today.
So, I'm changing the title of this issue to reflect only the "-o isn't passing through file path" problem.
Works for me. I was going to submit a pull request for the doc change when I got home, but that saves me the trouble.
So I had some time to dig through this tonight and I think I found the problem. My Ruby knowledge is pretty basic at this point but I think I got the gist of what's going on.
When combining --output with --m4v --mp4 or --mkv the path gets passed to filter_output_option(path, ext)
which contains this line:
File.basename(path, '.*') + ext
Unfortunately, that strips off any directory information from the path, leaving just the file name.
/Volumes/Converted/Hitchhiker.m4v becomes Hitchhiker.m4v.
I think the correct thing to do is something like this:
File.join(File.dirname(path), File.basename(path, '.*')) + ext
I couldn't find a way to get everything but the file's extension out of the File class, but this recreates the path based on the given directory and file name whatever extension is needed. There's probably a way to just swap out the extension using File.extname and some fancy string munging but this seemed like a pretty straightforward way to achieve the desired result in one line.
In working through this I also found what looks like a typo in an error message in resolve_output(media):
fail "output file exists: #{media.path}" if File.exist? output
should probably read
fail "output file exists: #{output}" if File.exist? output
since the code is looking at output for the failure condition.
Just to confirm, with the above change in place, running this command from my home directory:
./transcode-video.rb --title 12 --output /Volumes/Videos/Converted/HitchHiker.m4v --m4v --crop detect --add-audio 2 --copy-audio 2 --burn-subtitle scan --add-subtitle 1 --add-subtitle 6 --dry-run /Volumes/HGG0NND1/
Results in this output, which looks OK to me:
HandBrakeCLI --input=/Volumes/HGG0NND1/ --output=/Volumes/Videos/Converted/HitchHiker.m4v --markers --encoder=x264 --quality=16 --title=12 --crop=60:64:0:0 --strict-anamorphic --rate=23.976 --audio=1,1,2 --aencoder=ca_aac,copy,copy --audio-fallback=ac3 --subtitle=scan,1,6 --subtitle-burned --encopts=vbv-maxrate=2000:vbv-bufsize=1000:crf-max=25
So in all truth, calling --m4v on the command line was unnecessary as the scripts handle that already. However, I don't see any special handling for the case where someone enters an output file name with an extension and then uses --m4v, --mp4, or --mkv to specify an alternate extension (say --output test.m4v --mkv). The overridden extension is used (which makes sense) but a warning would probably be a good idea.
@arikalish That's some great debugging! Thanks! And you're right, that's the problem. I just confirmed. No idea why I couldn't reproduce that earlier. A pretty damn stupid bug, if you ask me.
And actually that's two bugs. You're right about the error message, too. Embarrassing. :)
My fix is slightly different than yours but it's all in the same place:
File.dirname(path) + File::SEPARATOR + File.basename(path, '.*') + ext
As for the warning you suggested, I don't think that's necessary. And in some cases where people are driving the tool via a script, it could be counterproductive.
I should have the fix checked in shortly and a new version posted sometime tonight. I'll respond here again when it's out.
Thanks again! You rock!
@arikalish Looks like that error message is also wrong in convert-video. Corrected.
@arikalish OK, version 0.2.6 should be available. Just do a gem update and let me know if it's still not working for you.
0.2.6 fixes the path issue. Thanks!
If I had to guess, I'd say you weren't adding the --m4v option when testing it out since you knew it wasn't really needed.
From the docs it wasn't totally clear to me that setting a file name would cause the script to use the correct output file type automatically or if it would always default to a Matroska file even when the file name ends with something else. Looking at the code it makes sense, though.
As for the warning: that's totally fine, I just worry that someone would specify a .m4v file name and then separately specify --mkv (or the reverse) and end up a bit surprised. Of course, if you're specifying the file name you don't really need to use the file type options anyway...
@arikalish Thanks for the testing and the feedback. I'll see what I can do to fix the built-in help and/or documentation to clarify the behavior of the --output option.