Custom ffmpeg command doesn't work due to it looking for quotes in the filename

Use this patch instead.

This was the custom command:

-i %in_file -vcodec libx264 -r 15 -b 1500k -vpre libx264-normal %out_file

The original $this->output_file extraction:

    // Find the output file in the command if there was one.
    // This is sort of hacky but is helpful for passing the file out.
    preg_match("/.*'(.*?)'.*?$/", $this->command, $matches);
    $this->output_file = ! empty($matches[1]) ? $matches[1] : NULL;

$this->output_file wasn't being extracted because there weren't quotes around the %outfile (in my case it was replacing %outfile with /tmp/videoname.ext)

Any chance this could get committed?

The problem with that is that $destination isn't set. Notice this, from ffmpeg_wrapper_convert_file():

  // build the output file path if we don't have one. Use the output type as the extension.
  if (! $destination = $params['destination']) {
    // Clean the destination path
    $pattern = "/[^a-zA-Z0-9\.]/";
    $filename = preg_replace($pattern, '_', basename($params['source']));
    // File name is set to the source plus the codec
    $filename .= '.' . $params['ffmpeg_output_type'];
    $destination = file_create_filename($filename, file_directory_temp());
  }

That's being triggered for me, because $params['destination'] isn't set. The -i syntax hasn't changed in the (many) years I've been using ffmpeg, and I think is safe to rely on.

If you look at the patch in #1 you'll see exactly what I'm doing: I'm replacing the current regex with better regex. (The current regex doesn't work for me because of the way the custom command is being passed to it.) Nothing else changes.

I'm a little confused - are you saying I should submit another patch that changes something else that bypasses this problem?

The current code does:

    <?php
    // Find the input file
    preg_match("/-i.*?'(.*?)'/", $this->command, $matches);
    $this->input_file = $matches[1];

    // Find the output file in the command if there was one.
    // This is sort of hacky but is helpful for passing the file out.
    preg_match("/.*'(.*?)'.*?$/", $this->command, $matches);
    $this->output_file = ! empty($matches[1]) ? $matches[1] : NULL;
    ?>

I'm simply replacing that with better regex.

I have no idea why/when/how $destination or $params['destination'] is (or is not) being set.

This patch just deals with the problem that the current module looks for quotes in $this->command, which isn't always the case.

And the $this->input_file relies on the -i syntax, so I think it's safe for my patch to as well.

Setting the input_file and output_file before calling command doesn't work, though I'm trying to track down why and will report back more info when I have it (even after commenting out the input_file and output_file in function command():

/usr/bin/ffmpeg -i sites/default/files/video.flv -vcodec libx264 -r 15 -b 1500k -y -vpre libx264-normal /tmp/video.flv_2.3g2 -v -1

Seems to be the same error where %out_file isn't being replaced.

OK, in ffmpeg_wrapper_ui_node_action_form, when it calls ffmpeg_wrapper_convert_file() it passes in $form_state['values'] which comes straight from the submitted form. ffmpeg_wrapper_convert_file then checks this:

if (! $destination = $params['destination']) {

Which will always be false in that case. Then it creates the destination based on the temp directory. So setting the output_file = $destination won't work because at that point it's already been set to /tmp/etc.

$params['destination'] will never actually be set:

$ grep -R "params\['destination'\]" *
ffmpeg_wrapper/ffmpeg_wrapper.module:  if (! $destination = $params['destination']) {

Unless someone does a _form_alter. How is it possible that this will ever output the file properly? (Actually very curious)

OK, so after spending the majority of today on this !@#%-ing problem, I have a proposed alternative.

My code happened to work, but only for presets. I propose that these four lines of code be added to ffmpeg_wrapper_ui_node_action_form and that the hacky solution for finding the input_file and output_file be removed, and your code preserved. Here are the four lines:

<?php
    if (empty($form_state['values']['destination'])) {
      $filename = preg_replace('/[^a-zA-Z0-9\.]/', '_', basename($form_state['values']['source'])) . '.' . $form_state['values']['ffmpeg_output_type'];
      $form_state['values']['destination'] = pathinfo($form_state['values']['source'], PATHINFO_DIRNAME) . '/' . $filename;
    }

    $ffmpeg_object = ffmpeg_wrapper_convert_file($form_state['values']);
?>

That will properly set the destination if it's not already set.

The above, in combination with your solution, will make everything work, for custom commands as well as normally.

The two modified patch files containing these changes attached.

Ok awesome, I'll download/test 6.x-2.x-dev and make sure it's working for me as well

I'm getting: Call to undefined function checkplain() in sites/all/modules/ffmpeg_wrapper/ffmpeg_wrapper_ui/ffmpeg_wrapper_ui.module on line 359

A quick fix to check_plain() should do it

This still isn't working for me since the destination isn't set. Can this be added to ffmpeg_wrapper_ui_configuration_form_validate, right before ffmpeg_wrapper_convert_file?

<?php
  
    if (empty($form_state['values']['destination'])) {
      $filename = preg_replace('/[^a-zA-Z0-9\.]/', '_', basename($form_state['values']['source'])) . '.' . $form_state['values']['ffmpeg_output_type'];
      $form_state['values']['destination'] = pathinfo($form_state['values']['source'], PATHINFO_DIRNAME) . '/' . $filename;
    }
?>

Damn, the destination still isn't being set:

There were errors during the conversion process
FFmpeg failed to create an output file. This could be due to permission problems or an error from the encoding settings.
Command: /usr/bin/ffmpeg -i sites/default/files/video.flv -vcodec libx264 -r 15 -b 1500k -y -vpre libx264-normal /tmpvideo.flv.mp4 -v 1

Using the same command as before:

-i %in_file -vcodec libx264 -r 15 -b 1500k -y -vpre libx264-normal %out_file

It looks like there should be an extra slash in there after /tmp, but shouldn't it be replacing %out_file with sites/default/files/video.flv.mp4 instead of /tmp/video.flv.mp4 ?

As I understand it, ffmpeg_wrapper_ui_configuration_form_validate gets called when someone hits the Transcode button on node/%nid/ffmpeg/%fid. That will then pass $form_state['values'] to ffmpeg_wrapper_convert_file, which then checks the existence of $params['destination'] and if it's not set, it'll create one in the temporary directory. I think that's the crux of the problem as I can see it, since $form_state['values']['destination'] never gets set.

Ahhh, I get it! So yeah, then this would be the only change:

<?php
    $filename = file_directory_temp() . '/' . $filename . '.' . $params['ffmpeg_output_type'];
?>

To put the / back in there between /tmp and the filename. And then we're good to go I think (phew!). Thanks for working through this.

Has this been committed into the dev branch yet do you know?