Posted by anantagati on December 4, 2008 at 8:34am
4 followers
| Project: | Media Mover |
| Version: | 6.x-1.0-alpha1 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
In function mm_content_harvest() - "Harvest from the CCK field", there is a condition to harvest files from nodes which have been changed after last_start_time of active configuration:
$harvest_conditions = ' AND n.changed > '. ($running_config->last_start_time ? $running_config->last_start_time : 0);When you want to use following configuration, there will be problem because node is changed in step 3 and 4, so changed time will be bigger than last_start_time of configuration. Result is that file is again and again harvested.
1. Harvest from CCK (video file)
2. Upload it to S3
3. Store link to S3 to text field in same node
4. Change node status to published
Comments
#1
I confirm this behavior. Additionally, the resultant files (80 for one node during the past two days) are not actually attached to the field (an ImageField in this case. should it be a textfield?)
#2
So maybe check for the date of harvest instead of the node updated date in the uniqueness test?
#3
I suspect the files not being attached are actually because of #330649: Extra argument in mm_content_harvest() causing errors.
That's probably the best. Otherwise, you'd need to manually change the node updated date, which is bad practice.
#4
There is also problem that user can sometimes edit node, but change only title or description. In another case he can change file.
So for first case when he is changing something else than file, file should not be harvested.
We can do something like this:
1. compare node update time with 'date from 'media_mover_files' table, if node update is less we don't need to harvest
2. if file is filefield or imagefield: compare timestamp of harvest with timestamp of file from 'file' table
3. if it is textfield: compare with 'nid' and 'textfield' from node with 'nid' and 'harvest_file' from 'media_mover_files' table
This should help to only harvest files which has been changed from last harvesting.
#5
I'm going to implement #4, then address aaron's concerns in #2
#6
I already did little changed #4.
It now harvest from filefield and image. Needs to take care also for textfield.
#7
anantagati-
Thanks for the patch. It does work, however, it is also harvesting files that are just in the files table for me. I'll investigate a bit further.
#8
Ok here is a slightly modified version. Seems to work as expected for me.
#9
I found there should not be 'else' otherwise it will not check for configuration started by mm_auto_run module.
if ($nid) {$harvest_conditions = ' AND n.nid = '. $nid;
}
else {
// select nodes that have beend modified since the last time that this
// configuration was run
$harvest_conditions = ' AND n.changed > '. ($running_config->last_start_time ? $running_config->last_start_time : 0);
// select files to harvest have been modified since the last time that this configuration was run
$harvest_conditions .= ' AND f.timestamp > '. ($running_config->last_start_time ? $running_config->start_time : 0);
$harvest_conditions .= ' AND n.nid IS NOT NULL ';
}
#10
anatagati-
Are you suggesting this:
<?php
if ($nid) {
$harvest_conditions = ' AND n.nid = '. $nid;
}
// select nodes that have beend modified since the last time that this
// configuration was run
$harvest_conditions = ' AND n.changed > '. ($running_config->last_start_time ? $running_config->last_start_time : 0);
// select files to harvest have been modified since the last time that this configuration was run
$harvest_conditions .= ' AND f.timestamp > '. ($running_config->last_start_time ? $running_config->start_time : 0);
$harvest_conditions .= ' AND n.nid IS NOT NULL ';
}
?>
It seems to me that the n.changed has to be in the else clause of the if ($nid) - the use case is the following:
User uploads content, content is transcoded by MM.
User is unhappy with results.
User changes configuration
User uses the rerun option to reprocess file
In this case, $nid would be set, but if the node was not edited in the mean time, it would not be processed.
Or am I missing what you're trying to do?
#11
Then he can 'Empty configuration files' and all files will be again processed.
If there will be 'else' and we skip those additional $harvest_options user cannot change title or body without processing file which he didn't change.
#12
"Media Mover Auto Run: Auto run specific Media Mover configurations when a node is updated or inserted"
It seems for me as bug, if file will be again processed after each update (change node status to published, ...)
If there will be autorun set to configuration which will just upload file to S3 and then publish node. It will be harvested every time when configuration is run, even if user didn't do anything, because node will be changed in action for complete 'Action: Set node status'.
#13
@anantagati re: #11
The possibility of re-running a node is important. There can be many factors (php time outs for one) that could prevent a file from being completed. Being able to reprocess a node on demand is a feature that has quite a bit of future potential. Emptying the configuration (potentially 1000s of files) is not an option in this case.
I think the setup that I've created to do this is a bit hacky. Likely there needs to be one system for doing bulk processing and one for individual node processing- auto run and individual node processing for another for one, and the cron based bulk setup for another.
This is probably something to think about for 2.0, but that is a bit far away right now :)
re: #12
Yes you're right. I think the problem is the same here- the individual node running system that I have created is not the best. It needs to be able to do some kinds of uniqueness checking that fixes exactly the problem you are identifying
#14
@arthurf re: #13
I agree how important feature is it.
I am ok with current state now, because it is not problem for me to use standard way of launching configuration in cron.
But I think it is still better to not process files which has not been edited, and better solution is to give to user some checkbox or tab which will give him ability for reprocessing of specific node. At this moment administrators who have rights to administer media mover, can delete specific file and it will be reprocessed.
#15
This bug is the show-stopper for getting http://youdrup.com/ working w/ media mover as a demo for the DIWD conference on Wednesday: I need to harvest a video from a filefield and store thumbnail & processed movie in the other fields. The process works fine, as the files are being processed and stored in file directories. They just never make it to the respective fields.
I'm on a plane tomorrow, but will take a look again when I get into the hotel in the evening. Sorry I haven't had a lot of time to put into debugging. Thanks for all the great work on the module! I'll put in a good plug one way or another, as the module is great, and works in d5, and many parts (such as FTP) seem to work fine in d6. It would be nice to demo this functionality as well.
Thanks,
Aaron
#16
Aaron- I'm on it and will make it work for you for DIWD. What are the field types that you're storing in and can you send me your MM configuration so I can test out how you're doing things?
#17
Arthur,
Awesome! Definitely! I can give you admin access as well, if that would be helpful.
The harvest is from field_video, which is a file_field accepting .mov, .avi, .mpg videos.
The first listed process turns the video into a .flv file. (This part works.) It's supposed to store it in field_flash_video, another file_field accepting only .flv files. (This part doesn't work.)
The second listed process creates a thumbnail from file_field. (This part works.) It's supposed to store it in field_thumbnail (an ImageField). (This part doesn't work.)
<?php$configuration->harvest->module = 'mm_content';
$configuration->harvest->action = '1';
$configuration->harvest->configuration = array(
'mm_config_harvest_field' => array(
'field_video' => 'field_video'
),
'cid' => '1',
'module' => 'mm_content',
'action' => '1',
'verb' => 'harvest',
);
$configuration->process->module = 'mm_ffmpeg';
$configuration->process->action = '1';
$configuration->process->configuration = array(
'ffmpeg_output_type' => 'flv',
'ffmpeg_audio_advanced' => '1',
'ffmpeg_audio_ab' => '64k',
'ffmpeg_audio_ar' => '22050',
'ffmpeg_audio_acodec' => 'ac3',
'ffmpeg_video_advanced' => '1',
'ffmpeg_video_size' => '128x96',
'ffmpeg_video_size_other' => '',
'ffmpeg_video_fps' => '25',
'ffmpeg_video_br' => '50k',
'ffmpeg_video_vcodec' => 'flv',
'ffmpeg_time_advanced' => '0',
'ffmpeg_time' => '30',
'ffmpeg_video_custom' => '0',
'ffmpeg_video_custom_command' => '-i %in_file %out_file',
'ffmpeg_output_perms' => '0644',
'cid' => '1',
'module' => 'mm_ffmpeg',
'action' => '1',
'verb' => 'process',
);
$configuration->storage->module = 'mm_content';
$configuration->storage->action = '2';
$configuration->storage->configuration = array(
'mm_config_save_field' => 'field_flash_video',
'cid' => '1',
'module' => 'mm_content',
'action' => '2',
'verb' => 'storage',
);
$configuration->complete->module = 'mm_node';
$configuration->complete->action = '3';
$configuration->complete->configuration = array(
'complete_conditions' => array(
'published' => 'published'
),
'cid' => '1',
'module' => 'mm_node',
'action' => '3',
'verb' => 'complete',
);
$configuration->required = array('mm_content','mm_ffmpeg','mm_content','mm_node');
$configuration->name = 'Convert Video Upload';
$configuration->description = 'Convert Video Upload to a Flash video, and store in the Flash Video field.';
$configuration->start_time = '1228774022';
$configuration->last_start_time = '1228770422';
$configuration->status = 'stopped';
$configuration->settings = '';
$configuration->hierarchy->parent = '0';
?>
<?php$configuration->harvest->module = 'mm_content';
$configuration->harvest->action = '1';
$configuration->harvest->configuration = array(
'mm_config_harvest_field' => array(
'field_video' => 'field_video'
),
'cid' => '2',
'module' => 'mm_content',
'action' => '1',
'verb' => 'harvest',
);
$configuration->process->module = 'mm_ffmpeg';
$configuration->process->action = '2';
$configuration->process->configuration = array(
'thumb_dimensions' => '320x240',
'thumb_dimensions_other' => '',
'thumb_time' => '00:00:02',
'thumb_format' => 'jpg',
'cid' => '2',
'module' => 'mm_ffmpeg',
'action' => '2',
'verb' => 'process',
);
$configuration->storage->module = 'mm_content';
$configuration->storage->action = '2';
$configuration->storage->configuration = array(
'mm_config_save_field' => 'field_thumbnail',
'cid' => '2',
'module' => 'mm_content',
'action' => '2',
'verb' => 'storage',
);
$configuration->complete->module = 'mm_node';
$configuration->complete->action = '3';
$configuration->complete->configuration = array(
'complete_conditions' => array(
'published' => 'published'
),
'cid' => '2',
'module' => 'mm_node',
'action' => '3',
'verb' => 'complete',
);
$configuration->required = array('mm_content','mm_ffmpeg','mm_content','mm_node');
$configuration->name = 'Create Video Upload Thumbnail';
$configuration->description = 'This will create a thumbnail from a video upload.';
$configuration->start_time = '1228774022';
$configuration->last_start_time = '1228770422';
$configuration->status = 'stopped';
$configuration->settings->mma_file_perm = 0;
$configuration->settings->mma_file_mask = '0644';
$configuration->settings->mma_node_edit_item_show = 1;
$configuration->settings->mma_node_item_delete = 1;
$configuration->settings->mma_node_config_rss = 1;
$configuration->settings->mma_cron_notify = 0;
$configuration->settings->mma_cron_notify_email = '';
$configuration->settings->mma_cron_notify_time = '10';
$configuration->settings->mma_process_num = '0';
$configuration->settings->mma_storage_num = '0';
$configuration->settings->mma_complete_num = '0';
$configuration->hierarchy->parent = '0';
?>
Thanks,
Aaron
#18
For Drupal 6 and saving of thumbnail to imagefield can help this: http://drupal.org/node/292904
And changed code in mm_content module based on that can look something like this:
<?php
/**
* Save to the CCK field
* @param array $configuration is the configuration array
* @param array $file is the file array
*/
function mm_content_node_save($configuration, $file) {
// we have to have an NID
if ($file['data']['nid']) {
// get the node
$node = node_load($file['data']['nid']);
// get the field
$field = content_fields($configuration['mm_config_save_field']);
switch ($field['type']) {
case 'text':
$node->{$field['field_name']} = array(array('value' => $file['process_file']));
// save the node
node_save($node);
break;
// handle image field case
case 'image':
//$file['hold'] = 1;
mm_content_field_image($node, $field, $file, $configuration);
break;
}
// clear the cache
cache_clear_all('content:'. $node->nid .':'. $node->vid, 'cache_content');
// return the file
return $file['storage_file'] = $file['process_file'];
}
// set an alert
watchdog('mm_content', 'Media Mover did not pass a node ID for storing data in a CCK field: !file', array('!file', l(t($file['mmfid']), 'admin/media_mover/file/edit/'. $file['mmfid'])));
return false;
}
/**
* function to save image in image fields
*/
function mm_content_field_image(&$node, $field, $file, $configuration) {
// For field_file_save_file()
module_load_include('inc', 'filefield', 'field_file');
// Loop over all the field/filepath pairs and create files objects.
$files = array(
$field['field_name'] => array(
'filepath' => $file['process_file'],
),
);
if (!empty($files)) {
foreach ($files as $field_name => $file_info) {
// Load the field and figure out the specific details.
$field = content_fields($field_name, $node->type);
$validators = array_merge(filefield_widget_upload_validators($field), imagefield_widget_upload_validators($field));
$files_path = _import_widget_files_directory($field);
if (!$_file = field_file_save_file($file_info['filepath'], $validators, $files_path)) {
drupal_set_message('Node with nid: '.$node->nid ." has problem with '". $field_name ."' field, you will have to run this batch again.");
$problem = TRUE;
}
$node->files[$field_name] = $_file;
}
}
$files = $node->files;
// Get the filefield module to do the saving and marking the record as permanent.
if (!empty($files)) {
foreach ($files as $field_name => $file) {
if (isset($node->$field_name)) {
array_push($node->$field_name, $file);
}
else {
$node->$field_name = array(0 => $file);
}
}
}
$node = node_submit($node);
node_save($node);
}
?>
<?php/**
* Determine the widget's files directory
*
* @param $field CCK field
* @return files directory path.
*/
function _import_widget_files_directory($field) {
$widget_file_path = $field['widget']['file_path'];
if (module_exists('token')) {
global $user;
$widget_file_path = token_replace($widget_file_path, 'user', $user);
}
return file_directory_path() .'/'. $widget_file_path;
}
?>
#19
..... coming in under the wire.......
Ok, I took another stab at this this evening. I think I've gotten closer- I'm able to harvest from a CCK file field and store back into a CCK file field and image field. I used anantagati's code as a sign post and tried to simplify things down. I think I've got a workable solution for the interim- at the very least, it should serve to build from. It will hopefully be more abstractable once the quirks get ironed out.
I've also made some changes to mm_ffmpeg which are small, but I was having issues with the error checking, so I turned it off (hey, who needs error checking, right?). This may have been necessary because of my bunk ffmpeg install on my mac. I also made some minor changes to media_mover_api and mm_node.
Some notes:
* I added a file type check on the harvest from CCK. Perhaps unnecessary, but could be useful
* I added a "list files" option on the store on CCK. This will automatically list files.
* I'm not sure if it is my imagefield install, but I'm not seeing the media mover attached files on the node edit screen. For that matter, I'm not seeing any uploaded images on the node edit screen, so I'm guessing it is just my install.
* field_file_save_file() killed me, not sure why. It was not taking CCK or media mover paths as $dest destination. Interestingly, it would take them for the filefield (this is why there are two functions at the moment). I'm not sure if this is because the filefield paths were preexisting... perhaps more testing when I've had some sleep.
* I am working off a modified version of ffmpeg_wrapper that I haven't had time to cleanup and commit yet. There could be errors introduced as a result of this.
Code is in CVS
#20
Still not working. Additionally, FFMPEG is creating 0 MB files now. (Looks like it always did for FLV videos, and hadn't realized it, but now the thumbnails are 0 bytes as well, from the same source files I'd tested with before).
#21
Funny that I did more harm then good. I'll attack this tonight.
#22
Aaron- also, you can try a compiled version of ffmpeg that I provide- I've had numerous problems with ffmpeg, and find then generally it doesn't work with flvs, wmvs, etc, unless you compile it by hand. Debian/Ubuntu's apt-get versions, OSX's fink all are problematic. The only good success that I've had is the by hand route.
http://mediamover.24b6.net/ffmpeg
#23
Ok- I spent a good deal of time looking at this. In each case (harvest from, save to, images, videos) it is working for me (on ubuntu with my own FFmpeg). I'm thinking this is actually an ffmpeg issue- in your logs, you should see something like "mm_ffmpeg converted this file: ..." can you grab the ffmpeg command from that report, and on the command line enter the root of your drupal install. Run the command from there (if it complains about over writing the file, you might want to check if it's ok). I'm guessing you're going to see an error here.
I've done quite a bit of upgrading to the ffmpeg_wrapper module which is more competent in catching errors now. Unfortunately, I haven't pulled these changes up to D6, but I'll work on that early tomorrow.
I'd also suggest that if ffmpeg is throwing errors grab the ffmpeg binary from the link above if you are on a linux/unix system and see if it works any better for you.
#24
I found I had issue with harvesting.
When I harvested to new configuration or to configuration where I used 'Empty configuration files' everything worked good for first time, but when I tried to harvest after that, it didn't get any new files anymore.
#25
anantagati- I merged your changes into some of the ones that I had already done. Your mechanism of checking against existing files rather than the time stamp is much better. I think this would fix the issues with auto run that we discussed previously. Thank you for continuing to work on this- the help is greatly appreciated!
#26
Seems to be working.
#27
Automatically closed -- issue fixed for 2 weeks with no activity.