I'm using multiple upload.
If I choose to delete the original video after conversion, in _flashvideo_perform_postop() function the following code is executed
Code part1
1 if (flashvideo_variable_get($node_type, 'delete', 0) && get_file_ext($oldfile->filepath) != 'flv') {
2 // Delete the original file from the file system.
3 file_delete($oldfile->filepath);
4 // Delete the original file from the files table.
5 db_query("DELETE FROM {files} WHERE fid = %d", $oldfile->fid);
6 // Delete the original file from the appropriate table.
7 if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) { // Using FileField
8 // For single-upload file fields
9 if (db_column_exists($cck_table, $cck_original_video_field .'_fid')) {
10 db_query("UPDATE {". $cck_table ."} SET ". $cck_original_video_field ."_fid = NULL, ". $cck_original_video_field ."_list = NULL WHERE nid = %d", $oldfile->nid);
11 }
12 else { // For multi-upload file fields
13 db_query("DELETE FROM {content_". $cck_original_video_field ."} WHERE ".$cck_original_video_field ."_fid = %d", $oldfile->fid);
14 }
15 }
16 else { // They're using the core Upload module.
17 db_query("DELETE FROM {upload} WHERE fid = %d", $oldfile->fid);
18 }
19 // Delete the file from the flashvideo table.
20 db_query("DELETE FROM {flashvideo} WHERE fid=%d", $oldfile->fid);
21 // Allow other modules do do some delete operations.
22 module_invoke_all('flashvideo_delete_file', $oldfile, $node_type);
23 }
The line n°13 delete each entry from the cck original field.
When I delete the node, the hook flashvideo_nodeapi is executed.
Code part 2
function flashvideo_nodeapi(&$node, $op, $teaser) {
1 // Only if the node type is enabled.
2 if (flashvideo_variable_get($node->type, 'enable', 0)) {
3 switch ($op) {
......
......
4 case 'delete':
5 case 'delete revision':
6 db_query("DELETE FROM {flashvideo} WHERE nid = %d", $node->nid);
7 if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) {
8 // Names of the different video fields? Let's find out.
9 $cck_original_video_field = flashvideo_variable_get($node->type, 'cck_original_video_field', '');
10 $cck_finished_video_field = flashvideo_variable_get($node->type, 'cck_finished_video_field', '');
11 $cck_finished_thumbnail_field = flashvideo_variable_get($node->type, 'cck_finished_thumbnail_field', '');
12 $cck_table = 'content_type_'. $node->type;
13 // Grab all of the files associated with the node.
14 $video_files = array();
15 foreach ($node->$cck_original_video_field as $file) {
16 $video_files[] = $file;
17 }
$node->$cck_original_video_field on line 15 IS NULL because we deleted each record after a successfull conversion.
My fix is to add line 14-18
14 if(!empty($node->$cck_original_video_field)) {
15 foreach ($node->$cck_original_video_field as $file) {
16 $video_files[] = $file;
17 }
18 }
that is better to write line 14 in this way:
14 if(!flashvideo_variable_get($node->type, 'delete', 0)) {// If we wish to delete the original video
because if you uploaded an flv (Line 1 of Code part 1 of this issue) the $node->$cck_original_video_field is not empty because the original video is the converted video at the same time
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | flashvideo.patch | 1.34 KB | tagliavinid |
Comments
Comment #1
attheshow commentedI just tried setting my content type to delete original videos after conversion and didn't receive any errors when I deleted my multi-video test node.
Comment #2
tagliavinid commentedYou're right, sorry for the second time.
This error appears because I fixed another (I suppose) bug.
Now I try to explain everything.
After you save a node and everything is ok, the following functions is called.
Take a look at line 76
If this line returns true we go to line 90 (for multi-upload file fields)
on line 90 $node->nid IS NULL because we pass as argument to this functions (_flashvideo_perform_postop) only $node->type (line 1, function declarations) and $node object doesnt' exists.
But even if we pass $node instead of $node->type, this make no sense, because in this way we delete all original video record in {content_". $cck_original_video_field ."} and not only that one we finished to process (if I have for example 3 videos in my node with multiple field upload).
For this reason when conversion is finished {content_". $cck_original_video_field ."} table is not empty, but contains my original videos record.
When I delete the node there are filefield hook that delete records inside the above table (e.g. filefield_field_delete hook).
When I found this bug I fixed line 90 in this way
This fix deleted orginal video record after a successful conversion.
And after all this we go back to the origin of my issue.
With your original code when I delete a node on line 15 $node->$cck_original_video_field I have an array with 3 NULL entry, one for each video (0=> NULL, 1=>NULL, 2=>NULL).
and going on the code doesn't produce error.
For example line 6 and line 8 may return simply false
In my case {ffmpeg_data} table grow up even if I delete node.
With my fix for line 90, the problem on this code (line 15 flashvideo_nodeapi in this issue) is that $node->$cck_original_video_field is not an array, but is a NULL value.
So finally we are at the initial reason for which I wrote this fix.
So, if you think my fix for line 90 (_flashvideo_perform_postop) is correct, you need a fix for line 15 of flashvideo_nodeapi too.
Hope this helps
Comment #3
tagliavinid commentedComment #4
attheshow commentedAny chance you can post a patch instead of such long strings of code? That would cover your suggested code changes in much more efficient manner. http://drupal.org/patch/create
Comment #5
tagliavinid commentedSorry, I'm new to drupal and his community, so I need more time to understand how to help people in the best way.
In this issue I reported all this code to help you to understand what I mean even if I changed only 3 lines.
Let's go on.
here is the patch.
The explanation why flashvideo needs this patch (in my opinion) is in this issue.
Comment #6
tagliavinid commentedComment #7
attheshow commentedJust fixed these error messages in last commit. Made sure that the item was an array before running foreach().