Video files "randomly" deleted

I have the same issue. I thought it might be related to the multiple attachment bug (so i patched that).. but it still seems to happen even without editing the video. I check the File attachment section and the attachment seems to be referencing a file with an appended "_0" at the end of it which might be the problem.

Any solutions?

I hope 5.0 solves this!

If video files are getting deleted unexpectedly then this is quite a serious issue. Has anybody come up with a solution for this?

Ok, I think I have a fix here. Testing would be greatly appreciated, but I've stopped files being deleted, and also stopped things duplicated as attachments and sometimes becoming renamed strangely which was leaving the links broken. If I've debugged and fixed this correctly then it appears all that was causing these nasty issues was a rogue session variable and a command which deleted whatever was left over in that variable, for whatever reason, when a separate node was being edited.

Attached is a detailed stack trace (generated using trace_backtrace() from the Trace module) of what happens when one clicks on the "edit" tab of a video node for the second time, which results in the video file being deleted in the spurious fashion described by Chris and Dan. The trace includes the full chain all the way to file_delete(), as well as the state of the $_SESSION superglobal.

Testing out Dan's patch now...

Thank you guys for your work on this bug.

I still did not tested the patch geodaniel provided, however I had I brief look are the code.

You removed the file_delete() call which is obviously what is causing problems here.
However it is there for a reason: remove those video which were uploaded but the video node
is never been submitted.

So.. it should work like this: if a user upload a video by using a preview but he never submit the node
then the next time the user open the video creation page it should remove the pending video which
is still on the webserver filesystem.

Removing file_delete() call fix the random video removing however I would like to keep the pending
video removing feature it should provide.

Hope this helps.

Fabio

Thanks for the explanation, Fabio. Even if we fixed the random file deletion bug though, your logic for deleting non-saved videos would still cause problems.

Imagine a scenario where somebody uploads a video, previews it and leaves that window in the preview page. In a different window he then uploads another video. The first one would still be deleted. Also, if the user doesn't go to create or edit another video during that session then the file will not be deleted anyway.

I think we need to find a different solution to deleting non-saved nodes. Perhaps a cron job that checks all the files and removes any that aren't linked to nodes? That could cause problems too though, if it is run between the user previewing a node and saving it.

Perhaps we could remove the buggy deletion code for now, work on a better solution for it, and implement that feature when ready? The way I see it, the bug is critical and the feature is a nice-to-have.

Fabio, I have to agree with Dan: one way or another, this should get patched up soon, as it's a super-critical bug. There's always time for the nice-to-have feature later...

Unfortunately I was having problems with geodaniel patch.
So.. I started working on this bug.

Attached is a patch which should fix some of the issues.
(this is testing code with debugging output)

I'm quite near fixing this one... but I need help in testing all the options.
It would be if you could test with this one and report the details
of what to do in order to reproduce the incorrect behaviour.

Fabio

Just posted to keep an eye on this. Aside from no sound, I am having the same problem as above.

Just trying this patch out now and it's still randomly deleting things - just at different times than before :(

Sets to reproduce:

1. Create video
2. Preview
3. Save
4. Click edit
5. Create new video

Original file is deleted (files/videos/apes.mp4 deleted prepare 1)

If videos are being put into the temp directory on preview, and only moved to the main videos directory when the node is saved, then surely things should only be allowed to be deleted from the temp directory? There should never be a need to delete things from the main videos directory unless someone explicitly requests it by deleting the attachment, or deleting the whole node.

Oops, that'd be 'steps' to reproduce :)

Not sure if this is related, but the thumbnail images remain in the temp folder even if the node is deleted - they are, however, removed from the main images folder upon deletion of the node. Not sure if there is a reason for this, but it seems odd.

It sounds related, but should be a feature request ticket for the video_image plug-in I think. This thread is concentrated on the video_upload.module and unexpected deletion of uploaded videos.

Fabio, what problems were you experiencing with the patch I provided? It's possible I overlooked a scenario when testing it.

I'd really like to get this bug out of the way, it's causing big problems for some of our users...

For the deletion feature of unused videos, I'm still trying to think of a sensible way of doing it. Current thinking is to maybe have a cron function that checks for unused videos left in the temp/video directory and if they are older than a few hours, then it could be presumed that it's safe to delete them. I don't think this could be done on prepare, as a) the user may not submit another video during the same session, b) they may submit a video before the few hours have passed.

I'd like to confirm the severity of this issue as my website just launched today and my logs have shown that I've greeted 300 video clicks with a blank screen due to the issue. My logs have traced the file deletion to when I uploaded a new file and edited the content (it deleted the previous video entry straight from the file system).

However severe this issue is, I'd still like to say thanks to you all for contributing to patching this issue, especially fax8 who's helped me personally via email in the past.

God speed gentlemen. Video on demand is the jugular of my website's success.

My steps to reproduce.:

1. Upload video and submit
2. Preview video
3. Edit a previously uploaded video and submit
4. Uploaded video in step 1 is deleted

After the geodaniel fix, everything seems fine.

With Fabio's patch:
My steps to reproduce:

Create a video.
Edit that video, do not hit submit.
Click on create a video.
Previous video is deleted.

With the fabio patch used the video file is deleted. I have ended up commenting out all instances of file_delete($_SESSION['video_upload_file']->filepath); and this has put a stop to it. However this obviously isnt a good fix since it will leave files on the server that shouldn't be there. I guess this is basically the same as geodaniel patch, since his removes that line of code as well and instead just unsets the path. This really does need to be fixed.

The attached patch should fix the problems reported with my previous patch.

Please test this out. Please check what happens to the temp video upload folder and the
definitive video folder.

This are the cases when video should be deleted:

• video creation page, upload video, preview, not submit, go to the video creation page again. pending video should be deleted
• video with uploaded file already created, edit the video node, upload another video, (optional: preview), submit. first video should be deleted
• video node deleted

Did I miss some cases?

my patch above is for 4.7. Video module for 5 will be different in implementation.

Thanks for updating the patch, Fabio. I've not had a chance to test it thoroughly yet but it's looking a bit better.

I'm still concerned about the first one on the list there about a non-submitted video being deleted the next time the node/add/video page is loaded. I like to work in tabs, so I might preview one video and go to open another video creation page in a new tab before I submit the first. With the latest patch, that first video that I'm previewing in the first tab would still get deleted. I think the safest way to handle this non-used video deletion would be with a delayed cron job (as noted in #15).

I also noticed something a couple of other strange things when working through a few test cases: 1) if you stop the browser whilst the file is uploading, the video node still gets created but the download link just points to the files directory listing, and 2) the video metadata in the 'Upload video' collapsible box doesn't always show the correct video information (I haven't reliably replicated this yet, but I've seen it on a number of occasions already)

I should get some more time to test this a bit better tomorrow

This patch didn't apply cleanly to v1.9 2006/09/18 video.module. I have hand-applied it and it seems to fix the issue - i.e. after adding a video I can add body text and metadata and a thumbnail and the video is not deleted. However, the multiple attachment issue http://drupal.org/node/74170 has re-emerged.

has anyone tried this with drupal 5?

I tried manually applying the patch and now no files are showing in the file attachments but the videos are still showing and the files haven't been deleted.

alynner

I am using video 5.1 and found that it has this same issue. I rewrote it to get rid of the $_SESSION stuff altogether. If taking full advantage of the drupal hooks, there is no need for it anyway.

Here is a patch that applies to the 5.1 version of Video. It no longer randomly deletes files. Please test it out and let me know what you think.

One caveat, if you are using Video Image and Video Upload, the weight of Video Image has to be greater than Video Upload.

Hi Vernon,

thank you for your help.

There are some problems during node previews. Once the user select the preview button on the node submission page it generate the preview ... however the uploaded file seems to be lost.

In the current implementation it also displays some useful informations on the uploaded file such as the file name, size and other stuff..

Could you check this one?

Here is an updated patch that fixes several issues. Now it does use the $_SESSION variable, but only for non-critical data. In other words, clearing the session would only make the video information disappear from the display, but the video would still be saved properly.

It also uses the files table to store the file and only pass the fid around rather than the video path in a hidden variable. This reduces the possibility of problems caused by malicious hand crafted posts.

Hi Vernon,

really a good work.

Some little problems:

• The call to
  url(drupal_get_path('module', 'video_upload') . '/busy.gif')
  will only work if clean urls are enabled. You should use:
  base_path() . drupal_get_path('module', 'video_upload') . '/busy.gif'
• If a video has been uploaded the form group should be expanded
• If a video has been uploaded the form now lists uploaded file information. I think that the upload file field should be below the video informations. The interface should say that, if the user upload another video, the current one will no more be available
• I think that file path and file mime informations could be not be displayed... Those are useless and could confuse the beginner user.
• video_upload_cron(): you should also remove stuff from files_revision table
• video_upload_cron(): DELETE FROM {files} WHERE nid = 1 AND filepath = '%s' nid=1 ??? I don't think this will work.
• what happens to previously updated video once a user upload a new one? I'm ok to not delete them but those should deleted in _video_upload_delete once the node get deleted
• _video_upload_load_file: given that there is video_fid, it should be used to query the files table. Using a simple WHERE r.vid = $node->vid could produce strange results if any other module which uses files is used

The most important thing you forget is updating the existing nodes to match the new behavior.
You should provide some update hooks in .install files.

Another possible problem:

the user upload a video and submit the node so it is stored.
Then he edit his node: he just modify some things in the body.
The form however insert a value in Video File field so once he submit
the modifications the video file value is stored on the DB.
Everything will continue to work until the user upload a new file ...
then the video file url value will have more importance and will let the
newly uploaded video to be not displayed.

Fabio,

I agree with you on most of the stuff. I have modified my patch to implement these changes. However, we need to figure out what to do with the temporary files that get inserted into the files table. Until Drupal 6, which should have some much needed improvements in the file management area, it is really easy for image module to hose the entries we put in the files table (there is an open bug against Image module currently: http://drupal.org/node/133436). This is the reason for the nid=1 stuff in the files table handling (cron and nodeapi(prepare) code).

Since they are temporary files, they don't have a nid to associate them with. I chose 1 because I had to choose something. 0 doesn't work because image will currently delete all those. I am thinking something along the lines of doing what image does and tag the files with a filename in the files table that is really just a label -- like "video_upload_temp.$filename" since the filename can always be extracted via basename($filepath). Then we can do the delete in the files table with DELETE FROM {files} WHERE filename LIKE 'video_upload_temp.%'.

I would have already posted my new patch, but I ran into a big issue with video image that I am reworking. If you try to edit an existing video and replace the video by uploading a new one, but only hit preview and never hit submit, the thumbnail image will get replaced permanently.

I am also not exactly sure how the whole file revisions thing works. I have never worked with that before. What I get by looking at the schema, we should really be storing the {file_revisions} vid rather than the {files} fid. Am I not looking at this right?

Anyway, it may be a while before I get all this sorted out. :) I think I have most of the video image issues resolved, but there are still a few corner cases I need to deal with.

Fabio,

Here is a new updated patch. I think I covered all the points listed by your two posts. However, I am still not really sure how the file revisions stuff works. But the rest of it seems to work for me. My instinct is to commit it because it works and does not have any critical bugs (like what it is trying to fix). Then we can work out the kinks later (if there are any). Plus, I have some major changes pending in Acidfree that depend on Video working (the album contents view). Please test this patch out and let me know if there are any problems. I really want to get this committed as soon as possible. When it comes time to commit it, I can do it if you want since I can probably provide a better change log for the commit.

Thank you Vernon for your hard work.

Unfortunately seems that there still some problems:
* Create a video node using the upload feature
* Edit the node and upload a new file

The video_upload form stuff seems to display the newly uploaded informations
while the video file field point to the old video. The Upload module widget at the bottom
instead lists only the old file and the new one is not present.
If you play the video now it will plays the first file, not the new one.
Instead the thumbnail will use the new file.

As the file url is inserted into the video file field seems
that the problem I pointed out on #29 is still here.

Further work on the video upload module. For some reason, my brain has not been able to focus on this issue very well lately. It took me way too long to come up with a solution for how to fix things this time.

But I think all is working now. With upload module enabled, it shows all the past video files that were uploaded for each node. The vidfile field is empty when editing a node that currently has video_fid set. If the use supplies a url in the vidfile field, video_fid is unset (but the file is not deleted from the node) and the node will use the vidfile url as the video source. Then a user can also override the vidfile by uploading a new video file.

The video information and the vidfile now seem to be congruent.

I think you should change line 161 from:

if ($node->video_fid) {

into:

if ($node->video_fid || $_SESSION['video_upload_file_info']) {

without this validation on previously previewed videos would fail.

sorry.. I mean line 161 of video_upload.module

Found others:
* Thumbnails nodes are created even if the video_ffmpeg helper is not active
* Create a video with upload, edit, choose another video then preview, then submit. The node will still have the first video attached while the second is displayed it's information on the video form but is not listed on upload.module fields (nid field not updated on the files table?)

Fabio,

An updated patch that:

1. Changes line 161 of video_upload.module so validation of uploaded videos won't fail
2. Does not create thumbnails if there is no thumbnail image present
3. Shows the correct video information when previewing

Vernon, I really appreciate your hard work on this bug.
Unfortunately there are still some problems with the code.

• With video_image on, but ffmpeg_helper off, if you submit a video without attaching a thumbnails, video_image try to add the thumb to the node content. There are no images attached then nothing get displayed.
• Create a video using upload (uploading file A), edit it, upload a new file B, hit preview then hit submit. The video node will still plays file A even if editing the node display file B as selected video.

I imagine that you get tired with this bug.. so.. If you don't want to put efforts on this I can continue the work on this. Just tell me.

My work on the rewriting is now stopped because I'm waiting for this code to became stable so I can include it.. I will be happy to take over this bug .. so I can proceed and release the rewritten version.

Fabio

Fabio,

I swear I tested the create, edit, preview submit scenario... grrr. Well, this back and forth between us doesn't seem to be the most time effective way of doing development. Why don't you go ahead and take the bug and run with it. Please let me know before you commit anything so I can check it out as well. (just posting to this bug is sufficient.)

Thanks. And good luck. At times, thing get a little tricky. :)

Ok.. I'm taking over this. I already started merging your last patch code with my rewrite.
As soon as I have something interesting I will post here.

Fabio,

Is seems as though the video module download has been updated as of June 20, 2007? Have you fixed this problem officially now and just not posted such? I'm having this same problem and am sincerely hoping this issue has been resolved with this last module update on 06/20/07. Please advise if at all possible, thank you!

Regards,
Joel

Joel8,

No, this bug has not been fixed yet.

Vernon,

I was eagerly hoping...

WOW you've contributed quite a bit here. Thank you! I guess at this time I can only apply your latest patch and see if that works for me by step test retracing. I would love to contribute however I'm not nearly as advanced as Fabio and yourself in this area.

Once I had seen the upload module updated 06/20/07, I immediately re-installed but my problem still exists (same as yours - files deleting based on edits). So I'll now go ahead and apply your patch and test... I'll post the outcome once done, probably within the next 48 hours..

I'm naturally anxious to get past this issue as it's holding up my client's launch date. I could never take this live with the module operating this way..

Regards,
Joel

This patch is a cleanup of the patch provided by #37 submitted by vernon (do_not_use_session_variable_3.patch). It doesn't work either, but at least it's "clean".

The main problems with it are:

1. the node->video_upload_file_path doesn't get passed around properly so the video doesn't get properly identified and the vidfile doesn't get initialized
2. it was not really tested outside of our setup here (ffmpeg + auto thumbnails)
3. we've just reverted to the old SESSIONS behaviour because it "works"

I do have to say that using sessions for upload management is the root cause for all the problems surrounding this bug, which I can confirm and suffer the consequences. In short, it's a really bad idea.

The main motivation in rewriting the patch from vernon was that applying it caused all our videos to stop working spontaneously. The attached files disappeared for some reason and nothing would get displayed. Also, the way we're setup, it was impossible for people to just link to outside videos: ffmpeg would try to transcode outside urls, which doesn't work....

anarcat,

The patch that I wrote is currently being fixed up and integrated into some major changes that Fabio is working on. The patch you supplied did not fix any of the problems with video_image and ffmpeg_helper as they relate to the changes that removing the _SESSION stuff included.

Originally I had removed all the _SESSION stuff, but it turns out that with Drupal 5's formAPI, it is impossible to get all the required information to display in the form without using _SESSION. But keep in mind that the information stored in the session is no longer critical data -- it is merely there for a better looking form (video information file size, etc.).

Please be patient and I am sure that Fabio will post something soon.

The patch that I wrote is currently being fixed up and integrated into some major changes that Fabio is working on. The patch you supplied did not fix any of the problems with video_image and ffmpeg_helper as they relate to the changes that removing the _SESSION stuff included.

I know and understand. I was just trying to share my work.

Originally I had removed all the _SESSION stuff, but it turns out that with Drupal 5's formAPI, it is impossible to get all the required information to display in the form without using _SESSION. But keep in mind that the information stored in the session is no longer critical data -- it is merely there for a better looking form (video information file size, etc.).

I have strong doubts about this premise. Can't file_check_upload be used where necessary? Or the information simply stored in the node?

Using sessions will always create problems.

Please be patient and I am sure that Fabio will post something soon.

I'll try. ;)

vernon, anarcat: I mailed you my last development version of the rewrite.
In the new video_upload module under types/ you should find the solution to this issue.

I've been able to fix it without using the $_SESSION, just some tricky form/nodeapi interactions.
I admit that it was really complex to track down all the form/nodeapi interactions but I think I have something
clean and good now.

As this fix is part of a bigger rewrite, I'm not able to commit anything before the rewrite is complete.
I hope to complete everything as soon as possible.

For anyone interested in the development have a look at http://www.varesano.net/blog/fabio/video+module+major+rewrite+status

If anyone would like to use the (hopefully) bugfree code in my rewrite and create a patch against current DRUPAL-5 version it would really useful.
I'm unable to do this as I'm concentrating on the rewrite. Any help appreciated.

If I can suggest something, I think you should commit your dev version to HEAD and mark that branch as "developpement/alpha/unstable" code. Then it's easier for people to track your changes and merge them back with 5.x than if it's one big tarball.

Thanks for the tarball anyways. :)

You might find this post useful: http://www.varesano.net/blog/fabio/understanding+drupal+hook+nodeapi+exe...

When it comes to managing patches, I find quilt http://savannah.nongnu.org/projects/quilt/ to be an indispensable tool. That is one of the tools I used to generate this patch.

This posting is the diff between CVS and the tarball that Fabio posted on his website. Please download and test this. The more eyes we have on it, the sooner we can get it committed and released.

Vernon, the code I posted on my website is still in an early development stage. Still not ready for testing, I posted on my website just to give an idea of what I'm creating and to allow people to take the bugfixed code from the new video_upload to merge and fix this bug.

Just to let you understand how this code is stile in development: it still contains debugging prints in it.

If you users want to have an idea of what the next video module will be and maybe add comments or suggestions then please try the patch above.
Instead if you want stable code to use on your website do not use the patch above.

This thread seems to have gone cold. But, I am having the same "deletion" problem while running a completely current Drupal 5.2 / video.module setup. Is it possible to get a status update?

Thanks so much for an awesome module.

I second this, any news? Thanks!

Love the module!
but I'm still tring to work arround the deltion flow ... but somtime's a file still gets deleted :-/
any news ?

Hello,

This is indeed a critical bug, I have experienced loss of all my video files (!). Fortunately, we have backups.

I think a patch should be comited, even if it removes some features from the video module, at least it would avoid data loss.

I don't think the video module is usable curently without one of the patch applied.

That said, why is there a video upload module? Why don' t you use the file attach system provided by Drupal? It would be much easier to track bugs like this one.

Instead of using the fileupload module, there would simply be a checkbox below the video file path input stating :
[x] use the file attached to this node

User would check this box and attach the file using the file attach system provided for nodes.

What do you think? Did I miss something ? I can provide a patch with this functionality if requested.

philippejadin, thanks for reviving this thread and summarizing where things are at for us late comers. I would like to install this module for a website, but I scanned down through the discussion and it looks like the current version not ready to do so given this bug.

Is there anyone out there who could testify that one of the above patches that allowed them to successfully use this module on a production site?

Phillipe, I haven't worked with this module enough to understand exactly what you're proposing. Would this be a patch to the current video module or entirely new code? Either way, I'd be interested as it seems there isn't much of an alternative at this point.

Here here, thanks for giving us the lowdown.
Please consider writing a comment at my open letter to raise support for the module:

http://drupal.org/node/187697

Rereading what I wrote, I think it's a bit too discouraging for video users.

The problem with video file disapearing is caused by the video upload module. This module is part of the video module and is optional. It's a sub-module of video module

There is a bug in this video upload module that causes video file to be deleted when you edit a node previously created.

The short term solution is simple : disable video upload module, upload video files by hand (using ftp for example) and fill the "video path" by hand as well

In the longer term, I think the video upload "submodule" should disapear, and the video module could use Drupal upload system to ease file uploads.

@philippe: that is a potential workaround for the bug, but if you actually rely on the video_upload functionality, removing the file_delete line from the code will do the trick until something gets committed to fix this properly.

It's frightening that this critical bug is still open almost a year after it was opened. Think of all those videos that have been deleted accidentally in the meantime.

I second that, I just installed video uploaded a video and when I edited it I received the following error:

Your video has not been stored

even though I had just watched it play. I'm just wondering now if I should commit to this module or look at something else that can upload video (not embed from Youtube, etc.)
Thanks.

I have been in touch with Fabio (Fax8) and I understand he is now actively working on fixes for Video 5.x and a new dev for 6.x. I guess he now has a break between semesters at school or something like that.

In the meantime, he's got a temporary suggestion:

"I can suggest you a quick and dirty way to fix this error... open the video_upload.module file and comment (put a // as start of line) each occurrences of a call to the file_delete function. This will remove any file deletion performed by the video module."

Has been working for us at http://www.cctvcambridge.org for now, we'll wait as he works on permanent fixes and see what comes up in version 6.

finally fixed this on 5.x-dev .. please wait until we release a stable version to upgrade your websites.

Please use 5.x-dev only on testing sites.

Having a similar problem and waiting for a stable version... (subscribing)

This can't be closed!

Hey, any solution for Drupal 4.7?

I tried the solution of // to any file_delete() funciton, the nasty solution it's worst, now I can't upload any video.

Help! All my videos are deleted the same day! It's useless this module!

Now I have to teach FTP and make accounts, and other nasty homeworks to the people here!

Take a look at the Asset module, its doing a lot of really good work so far and will only get better. But not for Drupal 4.x :(

Gauss, time to upgrade to Drupal 5. It will only become harder for you to maintain your modules in 4.x as time goes on.

I don't know what went wrong, but now not a single video wants to play. I restored all video module folder and upload.module as well as all setings to enable the module and it's options and nothing. The player loads but not the video, however it's possible to download the video.

Any sugestion is aprecciated. Maybe restoring some database tables...?

please .. open a separate issue for this problem.