Fix sequencer disk cache not writing data
ClosedPublic
Actions

Authored by Richard Antalik (ISS) on Dec 30 2020, 1:36 AM.

Details

Reviewers

Lukas Stockner (lukasstockner97)
Sybren A. Stüvel (sybren)

Commits

rBe45630a99fc2: Fix sequencer disk cache not writing data

Summary

Error handling added in 512a23c3d61d caused that reading header of new
file failed, since it is empty.

Don't attempt to read header if file is empty. If header can not be read
anyway, try to delete the file. Add asserts, as this should never
happen.

Diff Detail

Repository: rB Blender

Event Timeline

Richard Antalik (ISS) created this revision.Dec 30 2020, 1:36 AM

Harbormaster completed remote builds in B11916: Diff 32330.Dec 30 2020, 1:36 AM

Richard Antalik (ISS) requested review of this revision.Dec 30 2020, 1:36 AM

Hm, deleting the file might make debugging tricky in the future, but I guess you could always comment it out.

In D9954#248538, @Lukas Stockner (lukasstockner97) wrote:

Hm, deleting the file might make debugging tricky in the future, but I guess you could always comment it out.

I think that not having a file that is disfunctional and must be deleted manually is more important. Deletion of file is step after you are stopped by assert. If there is reproducible case to debug I don't think this is much of a problem. If case isn't reproducible, you still have to find reason how this file ended up empty in first place and how to prevent it.

Peter Fog (tintwotin) added projects: Video Sequencer, VFX & Video.Jan 3 2021, 8:55 AM

@Lukas Stockner (lukasstockner97) thanks for reporting the issue, and thanks @Richard Antalik (ISS) for the patch.

I think it's pretty good as it is, and I've pointed out some simple improvements. I'll leave it up to @Richard Antalik (ISS) whether to include those or not.

source/blender/sequencer/intern/image_cache.c
521	It's better to have a meaningful message, something like: BLI_assert(!"unable to read disk cache header");
625–627	Add a comment that explains the size check, as it might not be obvious why it happens. Something like: /* BLI_make_existing_file() above may create an empty file. This is fine, don't atttempt reading the header in that case. */

This revision is now accepted and ready to land.Jan 8 2021, 2:22 PM

Closed by commit rBe45630a99fc2: Fix sequencer disk cache not writing data (authored by Richard Antalik (ISS)). · Explain WhyJan 10 2021, 4:54 PM

This revision was automatically updated to reflect the committed changes.

Richard Antalik (ISS) added a commit: rBe45630a99fc2: Fix sequencer disk cache not writing data.

Revision Contents
Changeset List

Path

Size

source/

blender/

sequencer/

intern/

image_cache.c

7 lines

Diff 32579

View Options

source/blender/sequencer/intern/image_cache.c

Fix sequencer disk cache not writing dataClosedPublicActions