From 70ba5000c115a6588a63e27cae0df01801c1d7e0 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Mon, 8 Oct 2012 14:20:59 +0200 Subject: [PATCH] Proper command escaping --- src/FFMpeg/FFMpeg.php | 134 +++++++++++++---------- src/FFMpeg/FFProbe.php | 25 +++-- src/FFMpeg/Format/Audio.php | 5 +- src/FFMpeg/Format/Audio/DefaultAudio.php | 2 +- src/FFMpeg/Format/Video/WebM.php | 4 +- 5 files changed, 96 insertions(+), 74 deletions(-) diff --git a/src/FFMpeg/FFMpeg.php b/src/FFMpeg/FFMpeg.php index 4b83e22..ffcd923 100644 --- a/src/FFMpeg/FFMpeg.php +++ b/src/FFMpeg/FFMpeg.php @@ -17,6 +17,7 @@ use FFMpeg\Exception\RuntimeException; use FFMpeg\Format\Audio; use FFMpeg\Format\Video; use Symfony\Component\Process\Process; +use Symfony\Component\Process\ProcessBuilder; /** * FFMpeg driver @@ -50,10 +51,10 @@ class FFMpeg extends Binary } $this->threads = (int) $threads; - + return $this; } - + public function getThreads() { return $this->threads; @@ -68,7 +69,7 @@ class FFMpeg extends Binary */ public function open($pathfile) { - if ( ! file_exists($pathfile)) { + if (!file_exists($pathfile)) { $this->logger->addError(sprintf('FFmpeg failed to open %s', $pathfile)); throw new InvalidArgumentException(sprintf('File %s does not exists', $pathfile)); @@ -118,26 +119,28 @@ class FFMpeg extends Binary */ public function extractImage($time, $output) { - if ( ! $this->pathfile) { + if (!$this->pathfile) { throw new LogicException('No file open'); } - $cmd = $this->binary - . ' -i ' . escapeshellarg($this->pathfile) - . ' -vframes 1 -ss ' . $time - . ' -f image2 ' . escapeshellarg($output); + $builder = ProcessBuilder::create(array( + $this->binary, + '-i', $this->pathfile, + '-vframes', '1', '-ss', $time, + '-f', 'image2', $output + )); - $this->logger->addInfo(sprintf('FFmpeg executes command %s', $cmd)); + $process = $builder->getProcess(); - $process = new Process($cmd); + $this->logger->addInfo(sprintf('FFmpeg executes command %s', $process->getCommandline())); try { $process->run(); } catch (\RuntimeException $e) { - + } - if ( ! $process->isSuccessful()) { + if (!$process->isSuccessful()) { $this->logger->addError(sprintf('FFmpeg command failed : %s', $process->getErrorOutput())); $this->cleanupTemporaryFile($output); @@ -161,7 +164,7 @@ class FFMpeg extends Binary */ public function encode(Audio $format, $outputPathfile) { - if ( ! $this->pathfile) { + if (!$this->pathfile) { throw new LogicException('No file open'); } @@ -188,33 +191,40 @@ class FFMpeg extends Binary */ protected function encodeAudio(Audio $format, $outputPathfile) { - $cmd = $this->binary - . ' -y -i ' - . escapeshellarg($this->pathfile) - . ' ' . $format->getExtraParams() - . ' -threads ' . $this->threads - . ' -ab ' . $format->getKiloBitrate() . 'k ' - . ' ' . escapeshellarg($outputPathfile); + $builder = ProcessBuilder::create(array( + $this->binary, + '-y', '-i', + $this->pathfile, + '-threads', $this->threads, + '-ab', $format->getKiloBitrate() . 'k ', + )); + + foreach ($format->getExtraParams() as $parameter) { + $builder->add($parameter); + } if ($format instanceof Audio\Transcodable) { - $cmd .= ' -acodec ' . $format->getAudioCodec(); + $builder->add('-acodec')->add($format->getAudioCodec()); } if ($format instanceof Audio\Resamplable) { - $cmd .= ' -ac 2 -ar ' . $format->getAudioSampleRate(); + $builder->add('-ac')->add(2)->add('-ar')->add($format->getAudioSampleRate()); } - $process = new Process($cmd); + $builder->add($outputPathfile); - $this->logger->addInfo(sprintf('FFmpeg executes command %s', $cmd)); + $process = $builder->getProcess(); + $this->logger->addInfo(sprintf('FFmpeg executes command %s', $process->getCommandLine())); + + echo $process->getCommandLine() . "\n\n"; try { $process->run(); } catch (\RuntimeException $e) { - + } - if ( ! $process->isSuccessful()) { + if (!$process->isSuccessful()) { $this->logger->addInfo(sprintf('FFmpeg command failed')); throw new RuntimeException(sprintf('Encoding failed : %s', $process->getErrorOutput())); } @@ -234,15 +244,17 @@ class FFMpeg extends Binary */ protected function encodeVideo(Video $format, $outputPathfile) { - $cmd_part1 = $this->binary - . ' -y -i ' - . escapeshellarg($this->pathfile) . ' ' - . $format->getExtraParams() . ' '; + $builder = ProcessBuilder::create(array( + $this->binary, '-y', '-i', + $this->pathfile + )); - $cmd_part2 = ''; + foreach ($format->getExtraParams() as $parameter) { + $builder->add($parameter); + } if ($format instanceof Video\Resizable) { - if ( ! $this->prober) { + if (!$this->prober) { throw new LogicException('You must set a valid prober if you use a resizable format'); } @@ -275,54 +287,62 @@ class FFMpeg extends Binary $width = $this->getMultiple($dimensions->getWidth(), 16); $height = $this->getMultiple($dimensions->getHeight(), 16); - $cmd_part2 .= ' -s ' . $width . 'x' . $height; + $builder->add('-s')->add($width . 'x' . $height); } } if ($format instanceof Video\Resamplable) { - $cmd_part2 .= ' -r ' . $format->getFrameRate(); + $builder->add('-r')->add($format->getFrameRate()); /** * @see http://sites.google.com/site/linuxencoding/x264-ffmpeg-mapping */ if ($format->supportBFrames()) { - $cmd_part2 .= ' -b_strategy 1 -bf 3 -g ' . $format->getGOPSize(); + $builder->add('-b_strategy') + ->add('1') + ->add('-bf') + ->add('3') + ->add('-g') + ->add($format->getGOPSize()); } } if ($format instanceof Video\Transcodable) { - $cmd_part2 .= ' -vcodec ' . $format->getVideoCodec(); + $builder->add('-vcodec')->add($format->getVideoCodec()); } - $cmd_part2 .= ' -b ' . $format->getKiloBitrate() . 'k' - . ' -threads ' . $this->threads - . ' -refs 6 -coder 1 -qmin 10 -qmax 51 ' - . ' -sc_threshold 40 -flags +loop -cmp +chroma' - . ' -me_range 16 -subq 7 -i_qfactor 0.71 -qcomp 0.6 -qdiff 4 ' - . ' -trellis 1 -qscale 1 ' - . ' -ab 92k '; + $builder->add('-b')->add($format->getKiloBitrate() . 'k') + ->add('-threads')->add($this->threads) + ->add('-refs')->add('6')->add('-coder')->add('1')->add('-qmin') + ->add('10')->add('-qmax')->add('51') + ->add('-sc_threshold')->add('40')->add('-flags')->add('+loop') + ->add('-cmp')->add('+chroma') + ->add('-me_range')->add('16')->add('-subq')->add('7') + ->add('-i_qfactor')->add('0.71')->add('-qcomp')->add('0.6') + ->add('-qdiff')->add('4') + ->add('-trellis')->add('1')->add('-qscale')->add('1') + ->add('-ab')->add('92k'); if ($format instanceof Audio\Transcodable) { - $cmd_part2 .= '-acodec ' . $format->getAudioCodec(); + $builder->add('-acodec')->add($format->getAudioCodec()); } $tmpFile = new \SplFileInfo(tempnam(sys_get_temp_dir(), 'temp') . '.' . pathinfo($outputPathfile, PATHINFO_EXTENSION)); - $passes = array(); + $pass1 = $builder; + $pass2 = clone $builder; - $passes[] = $cmd_part1 . ' -pass 1 ' . $cmd_part2 - . ' -an ' . escapeshellarg($tmpFile->getPathname()); + $passes[] = $pass1 + ->add('-pass')->add('1')->add('-an')->add($tmpFile->getPathname()) + ->getProcess(); + $passes[] = $pass2 + ->add('-pass')->add('2')->add('-ac')->add('2') + ->add('-ar')->add('44100')->add($outputPathfile) + ->getProcess(); - $passes[] = $cmd_part1 . ' -pass 2 ' . $cmd_part2 - . ' -ac 2 -ar 44100 ' . escapeshellarg($outputPathfile); + foreach ($passes as $process) { - $process = null; - - foreach ($passes as $pass) { - - $this->logger->addInfo(sprintf('FFmpeg executes command %s', $pass)); - - $process = new Process($pass); + $this->logger->addInfo(sprintf('FFmpeg executes command %s', $process->getCommandline())); try { $process->run(); @@ -336,7 +356,7 @@ class FFMpeg extends Binary $this->cleanupTemporaryFile(getcwd() . '/av2pass-0.log'); $this->cleanupTemporaryFile(getcwd() . '/ffmpeg2pass-0.log.mbtree'); - if ( ! $process->isSuccessful()) { + if (!$process->isSuccessful()) { $this->logger->addInfo(sprintf('FFmpeg command failed')); throw new RuntimeException(sprintf('Encoding failed : %s', $process->getErrorOutput())); } diff --git a/src/FFMpeg/FFProbe.php b/src/FFMpeg/FFProbe.php index 1fc0aca..65ddabd 100644 --- a/src/FFMpeg/FFProbe.php +++ b/src/FFMpeg/FFProbe.php @@ -14,6 +14,7 @@ namespace FFMpeg; use FFMpeg\Exception\InvalidArgumentException; use FFMpeg\Exception\RuntimeException; use Symfony\Component\Process\Process; +use Symfony\Component\Process\ProcessBuilder; /** * FFProbe driver @@ -38,9 +39,11 @@ class FFProbe extends Binary throw new InvalidArgumentException($pathfile); } - $cmd = $this->binary . ' ' . escapeshellarg($pathfile) . ' -show_format'; + $builder = ProcessBuilder::create(array( + $this->binary, $pathfile, '-show_format' + )); - $output = $this->executeProbe($cmd); + $output = $this->executeProbe($builder->getProcess()); $ret = array(); @@ -84,9 +87,11 @@ class FFProbe extends Binary throw new InvalidArgumentException($pathfile); } - $cmd = $this->binary . ' ' . escapeshellarg($pathfile) . ' -show_streams'; + $builder = ProcessBuilder::create(array( + $this->binary, $pathfile, '-show_streams' + )); - $output = explode("\n", $this->executeProbe($cmd)); + $output = explode("\n", $this->executeProbe($builder->getProcess())); $ret = array(); $n = 0; @@ -123,28 +128,26 @@ class FFProbe extends Binary /** * - * @param string $command + * @param Process $process * @return string * @throws RuntimeException */ - protected function executeProbe($command) + protected function executeProbe(Process $process) { - $this->logger->addInfo(sprintf('FFprobe executes command %s', $command)); + $this->logger->addInfo(sprintf('FFprobe executes command %s', $process->getCommandline())); try { - $process = new Process($command); - $process->run(); } catch (\RuntimeException $e) { $this->logger->addInfo('FFprobe command failed'); - throw new RuntimeException(sprintf('Failed to run the given command %s', $command)); + throw new RuntimeException(sprintf('Failed to run the given command %s', $process->getCommandline())); } if ( ! $process->isSuccessful()) { $this->logger->addInfo('FFprobe command failed'); - throw new RuntimeException(sprintf('Failed to probe %s', $command)); + throw new RuntimeException(sprintf('Failed to probe %s', $process->getCommandline())); } $this->logger->addInfo('FFprobe command successful'); diff --git a/src/FFMpeg/Format/Audio.php b/src/FFMpeg/Format/Audio.php index cddf1f5..10e5e50 100644 --- a/src/FFMpeg/Format/Audio.php +++ b/src/FFMpeg/Format/Audio.php @@ -27,10 +27,9 @@ interface Audio public function getKiloBitrate(); /** - * Give som extra parameters to add to ffmpeg commandline - * Parameters MUST be escaped + * Return an array of extra parameters to add to ffmpeg commandline * - * @return string + * @return array() */ public function getExtraParams(); diff --git a/src/FFMpeg/Format/Audio/DefaultAudio.php b/src/FFMpeg/Format/Audio/DefaultAudio.php index 0dfd57e..7eaa5dd 100644 --- a/src/FFMpeg/Format/Audio/DefaultAudio.php +++ b/src/FFMpeg/Format/Audio/DefaultAudio.php @@ -31,7 +31,7 @@ abstract class DefaultAudio implements Resamplable, Interactive */ public function getExtraParams() { - return ''; + return array(); } /** diff --git a/src/FFMpeg/Format/Video/WebM.php b/src/FFMpeg/Format/Video/WebM.php index dc4a78d..fbb3abc 100644 --- a/src/FFMpeg/Format/Video/WebM.php +++ b/src/FFMpeg/Format/Video/WebM.php @@ -28,13 +28,13 @@ class WebM extends DefaultVideo { return true; } - + /** * {@inheritDoc} */ public function getExtraParams() { - return '-f webm'; + return array('-f', 'webm'); } /**