Hi, it would be a great feature to have hooks on conversion error and success in my opinion.

I´ve created a diff-patch for this but it needs a review.

--- ffmpeg_converter.module (saved version)
+++ (current document)
@@ -352,6 +352,7 @@
   // Run conversion and check result.
   $result = ffmpeg_converter_convert($input_file, $output_file, $preset->ffmpeg_wrapper, l(t('Source node'), 'node/' . $node->nid));
   if (!$result) {
+    module_invoke_all('conversion_error', $node, basename($input_file)); // Pass node and filename for identification.
     return false;
   }
   
@@ -397,6 +398,7 @@
   watchdog('ffmpeg_converter','FFmpeg converted the file %file to @format.',
            array('%file' => basename($input_file), '@format' => $preset->ffmpeg_wrapper['ffmpeg_output_type']),
            WATCHDOG_NOTICE, l(t('Source node'), 'node/' . $node->nid));
+  module_invoke_all('conversion_success', $node, basename($input_file)); // Pass node and filename for identification.
 }
 
 /**

//Kleve - Andreas Nilsson

CommentFileSizeAuthor
#7 ffmpeg_converter_1039172_hooks.patch1.98 KBzoo33
ffmpeg_converter-6.x-2.0-beta1.diff841 byteskleve

Comments

zoo33’s picture

zoo33 commented
Status: Needs review » Needs work

My apologies for the late reply.

This seems like a good idea. One thing I thought about is that the hook names probably need to begin with "ffmpeg_converter_" in order to not pollute the name space.

Can you give some more information on how you might use these hooks? Maybe provide an example implementation for each hook?

kleve’s picture

kleve commented

Hi

I've implemented the hooks in a project I am currently involved in. They are used to notify users via email when a conversion fails, and when the conversion is complete. Below is an example implementation of one of the hooks.

/**
 * Implementation of hook_conversion_error in ffmpeg_converter.
 */
function mymodule_conversion_error($node, $file_name) {
  $result = db_query('SELECT arguments FROM {job_queue}'); // If Job queue, get arguments containing nid for jobs
  $nr = 0;
  while ($row = db_fetch_object($result)) {
    $job = unserialize($row->arguments);
    if ($job[0] == $node->nid) {
      $nr++;
    }
  }
  if ($nr == 1) {
    $user = user_load($node->uid);
    if ($user->mail) { // Check if email address exists
      $params = array( // Body lines to add to the email
        'file' => t('The conversion could not be completed for the file '. $file_name),
        'delete' => t('You may try to upload the file again with different format and/or codec. To delete the unconverted video from MySite, you can use this link to access the video: mysite.com'),
      );
      // Send the email to trigger the hook_mail function
      drupal_mail('mymodule', 'mymodule_error', $user->mail, language_default(), $params, $from = 'NO-REPLY@mysite.com');
    }
    else {
      watchdog('mymodule', t('My Module was unable to send an error email to the user %user_name on conversion status for node %nid. E-mail address is missing.'), $variables = array('%user_name' => $user->name, '%nid' => $node->nid), $severity = WATCHDOG_ERROR, $link = NULL);
    }
  }
}
zoo33’s picture

zoo33 commented

Makes sense. Just a thought though, would it be better to implement this as two Rules events instead? Or maybe do both?

kleve’s picture

kleve commented

Both sounds nice. That way the information can be used in Rules and is also accessible to others who do not want to use the default Rules actions and/or configure new ones.

zoo33’s picture

zoo33 commented
Status: Needs work » Needs review

I've created a patch with modified versions of the hooks. I changed the arguments somewhat, plus made it so that $node is possible to pass by reference to hook implementations, which gives them the ability alter the node before it's saved.

I would love to hear you opinion and some test results!

I still think corresponding Rules events would be good, but maybe we should start with just the hooks.

kleve’s picture

kleve commented

This sounds great. Where can I have a look at your patch?

Also, It looks like you have assigned this issue to me or if I somehow managed to do this? Anyways, I am sorry but I did not notice this. That is why I have not done any modifications to my patch according to your previous comments.

zoo33’s picture

zoo33 commented
Assigned: kleve » Unassigned
StatusFileSize
new ffmpeg_converter_1039172_hooks.patch1.98 KB

I think you assigned yourself. :)

Forgot the patch, here it comes...

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 6.x-2.0-beta1 » 6.x-2.x-dev
Issue summary: View changes
Status: Needs review » Closed (outdated)
Issue tags: -confirmation message, -hooks (duplicate)

I am closing this issue, as Drupal 6 is no longer supported.