rsutphin / handbrake.rb

A ruby wrapper for HandBrakeCLI, a video transcoder
GNU Lesser General Public License v2.1
34 stars 6 forks source link

Spawning error on Windows. #3

Closed bmatsuo closed 13 years ago

bmatsuo commented 13 years ago

There seems to be problems using this gem on Windows.

I want people on Windows to be able to use the script I'm writing using handbrake.rb. But there seem to be some issues.

The major issue is that the windows shell does not accept commands/arguments that are single quoted.

mkdir -p .
Spawning HandBrakeCLI using "'HandBrakeCLI' '--input' 'The.Guild.S04E01-02.mkv' '--title' '1' '--aencoder' 'faac' '--mixdown' 'stereo' '--ab' '128' '--encoder' 'x264' '--encopts' 'ref=1:bframes=0:cabac=0:8x8dct=0:weightp=0:me=umh:subq=9:trellis=0' '--quality' '21.5' '--custom-anamorphic' '--keep-display-aspect' '--width' '640' '--height' '480' '--markers' '--ipod-atom' '--output' './The.Guild.S04E01.x264.mp4' 2>&1"
''HandBrakeCLI'' is not recognized as an internal or external command,
operable program or batch file.

Apparently windows only accepts double quotes, and if that wasn't bad enough, it has special escaping rules for nested quotes >_< http://technet.microsoft.com/en-us/library/cc723564.aspx I believe those are rules accurate. From a limited number of tests, they seem to work as advertised on my Win 7 installation. Although I'm pretty foreign to Windows development.

irb(main):017:0> IO.popen('"HandBrakeCLI" --help 2>&1') do |r|
irb(main):018:1*   while line = r.read(60)
irb(main):019:2>     $stdout.write(line)
irb(main):020:2>   end
irb(main):021:1> end
Syntax: HandBrakeCLI [options] -i <device> -o <file>

### General Handbrake Options------------------------------------------------

    -h, --help              Print help
    -u, --update            Check for updates and exit
    -v, --verbose <#>       Be verbose (optional argument: logging level)
...

The best fix I have found for determining the operating system Ruby is running on is through the 'rbconfig' http://www.ruby-forum.com/topic/86488 It seems to me like this module is included in Ruby 1.8.7

Windows systems, from what I've seen, have a target_os and host_os that matches the pattern /^mingw/ (Minimalist GNU for Windows) So, a simple check for this and an alternate quoting method should do the trick here.

I'd be happy to help if I can. Although I really don't know much about building gems. So I'd have to teach myself first.

If you're going to test this yourself, you should know that I got this far by in the process by installing the win32-open3 gem. Which is required on Windows as Ruby is lacking a native open3. So, if you were going to be checking for a windows operating system anyway, it might be a good idea to require "win32/open3" in the same spot. Then the users of handbrake.rb won't need to require it themselves.

Is it possible to have gem dependencies that are conditional on the operating system?

rsutphin commented 13 years ago

Is it possible to have gem dependencies that are conditional on the operating system?

It is possible, but it requires building separate versions of the gem on each targeted platform. I'm not interested in adding a dependency on Windows to my release process for this gem. I also think it would be irresponsible for me to accept patches to support Windows, because I'm not interested in testing on that platform — I couldn't make sure that they continued to work.

That said, HandBrake::CLI already has a feature that should allow you to get this to work. When you construct a new instance, one of the options you can pass in is :runner. This is a command object that receives the configured arguments and actually executes HandBrakeCLI.

Based on the research you've already done, it should be pretty easy to write a runner that works on Windows. Take a look at the default runner — HandBrake::CLI::PopenRunner — and write something that behaves the same but works on Windows.

bmatsuo commented 13 years ago

Ok. I can understand your trepidation about Windows patches. It's definitely adding some complexity to my script that is harder for me to test.

Thanks for your suggestion about a custom PopenRunner. Doesn't seem like it'll be hard. From what I see, seems it should only require that I override the command method.

Thanks a lot for your help, @rsutphin.

rsutphin commented 13 years ago

I think that subclassing might be more difficult than just replacing the runner. Right now, the default runner takes the HandBrake::CLI instance as a parameter. You won't be able to do that with a custom runner — the runner will have to be constructed before it is passed to HandBrake::CLI.new. I've entered #4 to address this.

bmatsuo commented 13 years ago

What I didn't realize at first is that HandBrake::PopenRunner is a private class and, therefore, I can't subclass it. I would like to though. Maybe when issue #4 is taken care of.

What I ended up doing was mostly copying the HandBrake::PopenRunner class, making @cli a writable attribute, and setting the the attribute after the HandBrake::CLI object is initialized.

hb = nil
r = MyBrake::WinPopenRunner.new(hb)
hb = HandBrake::CLI.new(:bin_path => hbbin, :runner => r)
r.cli = hb

It is sloppy. But it seemed to work out. Doesn't brake anything because the runner has nothing to do between being given a nil object and being given the initialized HandBrake::CLI instance.

rsutphin commented 13 years ago

What I didn't realize at first is that HandBrake::PopenRunner is a private class and, therefore, I can't subclass it.

It's not. (AFAIK, ruby doesn't have access control for classes or modules — just methods.) I can see two places where this confusion may have arisen:

bmatsuo commented 13 years ago

Oh, OK. That explains it. I didn't notice the scoping of the PopenRunner class. I just saw the @private tag and made a bad assumption when I hit snag >_< That will make some stuff simple.

Sorry I am kind of a Ruby noob. It's been years since I was really getting into it. And, this gem was the sole reason I'm writing Ruby right now. Thanks a lot for the help, @rsutphin ^_^