From 5b1126da664318689f8370ba3d9e6bdc5aa2a450 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Tue, 17 Jun 2025 14:15:19 +0200 Subject: [PATCH] Fix #140375: Image editor save overwrites movie file Create a good default name for saving individual frames of a movie file loaded as an image datablock, instead of the movie file name. Changes ImBuf to store the frame separate from the filepath, to implement this. Seems more clear for ImBuf.filepath to be an actual filepath anyway. Thanks to Jesse and Aras for investigating this bug. Pull Request: https://projects.blender.org/blender/blender/pulls/140471 --- source/blender/blenkernel/intern/image_save.cc | 9 +++++++++ source/blender/editors/space_image/image_undo.cc | 14 +++++++++----- source/blender/imbuf/IMB_imbuf_types.hh | 2 ++ source/blender/imbuf/movie/intern/movie_read.cc | 3 ++- source/blender/windowmanager/intern/wm_playanim.cc | 1 + 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/source/blender/blenkernel/intern/image_save.cc b/source/blender/blenkernel/intern/image_save.cc index 143bc74ca74..2599cec3830 100644 --- a/source/blender/blenkernel/intern/image_save.cc +++ b/source/blender/blenkernel/intern/image_save.cc @@ -109,6 +109,15 @@ bool BKE_image_save_options_init(ImageSaveOptions *opts, * by the image saving code itself. */ BKE_image_user_file_path_ex(bmain, iuser, ima, opts->filepath, false, false); + /* For movies, replace extension and add the frame number to avoid writing over the movie file + * itself and provide a good default file path. */ + if (ima->source == IMA_SRC_MOVIE) { + BLI_path_extension_strip(opts->filepath); + SNPRINTF(opts->filepath, "%s_%.*d", opts->filepath, 4, ibuf->fileframe); + BKE_image_path_ext_from_imformat_ensure( + opts->filepath, sizeof(opts->filepath), &opts->im_format); + } + /* sanitize all settings */ /* unlikely but just in case */ diff --git a/source/blender/editors/space_image/image_undo.cc b/source/blender/editors/space_image/image_undo.cc index 2ed9bf8dcb6..21ae2ea4b76 100644 --- a/source/blender/editors/space_image/image_undo.cc +++ b/source/blender/editors/space_image/image_undo.cc @@ -467,6 +467,7 @@ struct UndoImageBuf { UndoImageBuf *post; char ibuf_filepath[IMB_FILEPATH_SIZE]; + int ibuf_fileframe; UndoImageTile **tiles; @@ -498,6 +499,7 @@ static UndoImageBuf *ubuf_from_image_no_tiles(Image *image, const ImBuf *ibuf) MEM_callocN(sizeof(*ubuf->tiles) * ubuf->tiles_len, __func__)); STRNCPY(ubuf->ibuf_filepath, ibuf->filepath); + ubuf->ibuf_fileframe = ibuf->fileframe; ubuf->image_state.source = image->source; ubuf->image_state.use_float = ibuf->float_buffer.data != nullptr; @@ -667,10 +669,11 @@ static void uhandle_free_list(ListBase *undo_handles) static UndoImageBuf *uhandle_lookup_ubuf(UndoImageHandle *uh, const Image * /*image*/, - const char *ibuf_filepath) + const char *ibuf_filepath, + const int ibuf_fileframe) { LISTBASE_FOREACH (UndoImageBuf *, ubuf, &uh->buffers) { - if (STREQ(ubuf->ibuf_filepath, ibuf_filepath)) { + if (STREQ(ubuf->ibuf_filepath, ibuf_filepath) && ubuf->ibuf_fileframe == ibuf_fileframe) { return ubuf; } } @@ -679,7 +682,7 @@ static UndoImageBuf *uhandle_lookup_ubuf(UndoImageHandle *uh, static UndoImageBuf *uhandle_add_ubuf(UndoImageHandle *uh, Image *image, ImBuf *ibuf) { - BLI_assert(uhandle_lookup_ubuf(uh, image, ibuf->filepath) == nullptr); + BLI_assert(uhandle_lookup_ubuf(uh, image, ibuf->filepath, ibuf->fileframe) == nullptr); UndoImageBuf *ubuf = ubuf_from_image_no_tiles(image, ibuf); BLI_addtail(&uh->buffers, ubuf); @@ -690,7 +693,7 @@ static UndoImageBuf *uhandle_add_ubuf(UndoImageHandle *uh, Image *image, ImBuf * static UndoImageBuf *uhandle_ensure_ubuf(UndoImageHandle *uh, Image *image, ImBuf *ibuf) { - UndoImageBuf *ubuf = uhandle_lookup_ubuf(uh, image, ibuf->filepath); + UndoImageBuf *ubuf = uhandle_lookup_ubuf(uh, image, ibuf->filepath, ibuf->fileframe); if (ubuf == nullptr) { ubuf = uhandle_add_ubuf(uh, image, ibuf); } @@ -773,7 +776,8 @@ static UndoImageBuf *ubuf_lookup_from_reference(ImageUndoStep *us_prev, /* Use name lookup because the pointer is cleared for previous steps. */ UndoImageHandle *uh_prev = uhandle_lookup_by_name(&us_prev->handles, image, tile_number); if (uh_prev != nullptr) { - UndoImageBuf *ubuf_reference = uhandle_lookup_ubuf(uh_prev, image, ubuf->ibuf_filepath); + UndoImageBuf *ubuf_reference = uhandle_lookup_ubuf( + uh_prev, image, ubuf->ibuf_filepath, ubuf->ibuf_fileframe); if (ubuf_reference) { ubuf_reference = ubuf_reference->post; if ((ubuf_reference->image_dims[0] == ubuf->image_dims[0]) && diff --git a/source/blender/imbuf/IMB_imbuf_types.hh b/source/blender/imbuf/IMB_imbuf_types.hh index 79faffaf313..5e11c70d9f9 100644 --- a/source/blender/imbuf/IMB_imbuf_types.hh +++ b/source/blender/imbuf/IMB_imbuf_types.hh @@ -242,6 +242,8 @@ struct ImBuf { ImbFormatOptions foptions; /** The absolute file path associated with this image. */ char filepath[IMB_FILEPATH_SIZE]; + /* For movie files, the frame number loaded from the file. */ + int fileframe; /** reference counter for multiple users */ int32_t refcounter; diff --git a/source/blender/imbuf/movie/intern/movie_read.cc b/source/blender/imbuf/movie/intern/movie_read.cc index 1d096a6209c..6797822ec36 100644 --- a/source/blender/imbuf/movie/intern/movie_read.cc +++ b/source/blender/imbuf/movie/intern/movie_read.cc @@ -1432,7 +1432,8 @@ ImBuf *MOV_decode_frame(MovieReader *anim, #endif if (ibuf) { - SNPRINTF(ibuf->filepath, "%s.%04d", anim->filepath, anim->cur_position + 1); + STRNCPY(ibuf->filepath, anim->filepath); + ibuf->fileframe = anim->cur_position + 1; } return ibuf; } diff --git a/source/blender/windowmanager/intern/wm_playanim.cc b/source/blender/windowmanager/intern/wm_playanim.cc index 66806c13861..a120f0155a6 100644 --- a/source/blender/windowmanager/intern/wm_playanim.cc +++ b/source/blender/windowmanager/intern/wm_playanim.cc @@ -2015,6 +2015,7 @@ static bool wm_main_playanim_intern(int argc, const char **argv, PlayArgs *args_ #endif /* USE_FRAME_CACHE_LIMIT */ STRNCPY(ibuf->filepath, ps.picture->filepath); + ibuf->fileframe = ps.picture->frame; } while (pupdate_time()) {