Nice module and would be even more nice if the module could provide some hooks, for example the function ffmpeg_wrapper_convert_file() could provide a hook to alter the options array before calling $ffmpeg->command.

For example I'd like to have the functionality to downscale videos but not to upscale them. Maybe would be nice to have this as a feature of ffmpeg_wrapper, but if not I'd like to just but it in a custom module by implementing a hook like hook_ffmpeg_wrapper_convert_file_allter().

The options array should be an associative array to enable altering of the options like $options['-b'] = $params['ffmpeg_video_br'] or $options['ffmpeg_video_br'] = '-b ' . $params['ffmpeg_video_br'].

At the end I would also see some improvements in the specific ffmpeg comand the user could provide. It should be able to reflect some of the advanced parameters by the use of a placeholders like "-i %in_file -s %size_and_padding %out_file".

Want to hear the opinion of other users but especially of the maintainers if they would be willing to integrate such features.

Here are some more or less related issues:
#1202902: Add integration with ffmpeg_converter
#1236572: Use ffmpeg_wrapper_convert_file instead of ffmpeg_converter_convert
#1236746: ffmpeg_converter_convert is not feature complete to ffmpeg_wrapper
#806144: Invoke an alter hook for conversions

Comments

osopolar’s picture

Title: Make the module more extentable by providing somo hooks » Make the module more extentable by providing alter hooks
osopolar’s picture

Issue summary: View changes

a bit shorter

arthurf’s picture

I guess I'm a bit unclear on this issue- when would an alter hook need to fire? Since each module is responsible for calling ffmpeg_wrapper_convert_file() the settings for this command should already be correct- why would they need to be altered. While it's trivial to add this hook I'm unclear on what the use case actually is.

Looking at your request for "-i %in_file -s %size_and_padding %out_file". I can see why you'd want this alter but again I'm unclear as to why this wouldn't be set already coming into ffmpeg_wrapper_convert_file().

I think full token support should be available to the custom command which would mean adding token hooks.

As per $options['-b'] = $params['ffmpeg_video_br'] If you look at the D7 branch I've changed all the parameters to remove the 'ffmpeg_' prefix to make it easier to pass in the $params. I thought about using the explicit ffmpeg commands but it seemed to me that as an API layer it was better to passing more generic terms to start with- this would help future proof against changes to ffmpeg itself.

osopolar’s picture

@arthurf: what do you think about #1236572: Use ffmpeg_wrapper_convert_file instead of ffmpeg_converter_convert? Maybe we need the mentioned hooks to achieve it.

osopolar’s picture

Issue summary: View changes

Add example where to use the alter hook.