From 93be12fde050ed5a2102a0e01db659c97e318114 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Thu, 10 Jul 2025 17:39:00 +0200 Subject: [PATCH] ImBuf: Linearize float buffers from FFmpeg Blender expects float buffers to be in scene linear space, which was violated bu the 1012bit movie reading code. While such image buffers can be displayed correctly, performing operations in various areas of Blender might lead to unexpected results. The non-linear colorspace for ImBuf is expected to be "internal-only" to a specific area, like VSE. This is only done for Image and MovieClip data-blocks, sequencer still reads movie files in their original colorspace as it helps performance and sequencer can not be referenced from places where linear colorspace for float buffer is really important. Pull Request: https://projects.blender.org/blender/blender/pulls/141603 --- source/blender/blenkernel/BKE_image.hh | 4 ++- source/blender/blenkernel/intern/image.cc | 16 ++++++---- source/blender/blenkernel/intern/movieclip.cc | 2 +- .../space_sequencer/sequencer_drag_drop.cc | 4 ++- source/blender/imbuf/intern/thumbs.cc | 5 ++- source/blender/imbuf/movie/MOV_read.hh | 1 + .../imbuf/movie/intern/movie_proxy_indexer.cc | 7 ++-- .../blender/imbuf/movie/intern/movie_read.cc | 32 ++++++++++++++++--- .../blender/imbuf/movie/intern/movie_read.hh | 2 ++ .../sequencer/intern/cache/thumbnail_cache.cc | 3 +- source/blender/sequencer/intern/proxy.cc | 5 ++- source/blender/sequencer/intern/render.cc | 5 ++- source/blender/sequencer/intern/strip_add.cc | 24 ++++++++++---- source/blender/sequencer/intern/utils.cc | 4 +++ .../windowmanager/intern/wm_playanim.cc | 6 ++-- 15 files changed, 91 insertions(+), 29 deletions(-) diff --git a/source/blender/blenkernel/BKE_image.hh b/source/blender/blenkernel/BKE_image.hh index 85626acee5e..d2584e35ef8 100644 --- a/source/blender/blenkernel/BKE_image.hh +++ b/source/blender/blenkernel/BKE_image.hh @@ -137,12 +137,14 @@ bool BKE_imbuf_write_as(ImBuf *ibuf, * Used by sequencer too. */ MovieReader *openanim(const char *filepath, - int flags, + int ibuf_flags, int streamindex, + bool keep_original_colorspace, char colorspace[IMA_MAX_SPACE]); MovieReader *openanim_noload(const char *filepath, int flags, int streamindex, + bool keep_original_colorspace, char colorspace[IMA_MAX_SPACE]); void BKE_image_tag_time(Image *ima); diff --git a/source/blender/blenkernel/intern/image.cc b/source/blender/blenkernel/intern/image.cc index 188ac2847c1..a8957e8c4cc 100644 --- a/source/blender/blenkernel/intern/image.cc +++ b/source/blender/blenkernel/intern/image.cc @@ -2677,25 +2677,27 @@ bool BKE_imbuf_write_stamp(const Scene *scene, } MovieReader *openanim_noload(const char *filepath, - int flags, - int streamindex, + const int flags, + const int streamindex, + const bool keep_original_colorspace, char colorspace[IMA_MAX_SPACE]) { MovieReader *anim; - anim = MOV_open_file(filepath, flags, streamindex, colorspace); + anim = MOV_open_file(filepath, flags, streamindex, keep_original_colorspace, colorspace); return anim; } MovieReader *openanim(const char *filepath, - int flags, - int streamindex, + const int ibuf_flags, + const int streamindex, + const bool keep_original_colorspace, char colorspace[IMA_MAX_SPACE]) { MovieReader *anim; ImBuf *ibuf; - anim = MOV_open_file(filepath, flags, streamindex, colorspace); + anim = MOV_open_file(filepath, ibuf_flags, streamindex, keep_original_colorspace, colorspace); if (anim == nullptr) { return nullptr; } @@ -4145,7 +4147,7 @@ static ImBuf *load_movie_single(Image *ima, ImageUser *iuser, int frame, const i BKE_image_user_file_path(&iuser_t, ima, filepath); /* FIXME: make several stream accessible in image editor, too. */ - ia->anim = openanim(filepath, flags, 0, ima->colorspace_settings.name); + ia->anim = openanim(filepath, flags, 0, false, ima->colorspace_settings.name); /* let's initialize this user */ if (ia->anim && iuser && iuser->frames == 0) { diff --git a/source/blender/blenkernel/intern/movieclip.cc b/source/blender/blenkernel/intern/movieclip.cc index 4d93f86c2c2..732f46a4127 100644 --- a/source/blender/blenkernel/intern/movieclip.cc +++ b/source/blender/blenkernel/intern/movieclip.cc @@ -591,7 +591,7 @@ static void movieclip_open_anim_file(MovieClip *clip) BLI_path_abs(filepath_abs, ID_BLEND_PATH_FROM_GLOBAL(&clip->id)); /* FIXME: make several stream accessible in image editor, too */ - clip->anim = openanim(filepath_abs, IB_byte_data, 0, clip->colorspace_settings.name); + clip->anim = openanim(filepath_abs, IB_byte_data, 0, false, clip->colorspace_settings.name); if (clip->anim) { if (clip->flag & MCLIP_USE_PROXY_CUSTOM_DIR) { diff --git a/source/blender/editors/space_sequencer/sequencer_drag_drop.cc b/source/blender/editors/space_sequencer/sequencer_drag_drop.cc index 42e327b34b7..f7c320a3b8c 100644 --- a/source/blender/editors/space_sequencer/sequencer_drag_drop.cc +++ b/source/blender/editors/space_sequencer/sequencer_drag_drop.cc @@ -548,8 +548,10 @@ static void prefetch_data_fn(void *custom_data, wmJobWorkerStatus * /*worker_sta #endif } + /* The movie reader is not used to access pixel data here, so avoid internal colorspace + * conversions that ensures typical color pipeline in Blender as they might be expensive. */ char colorspace[/*MAX_COLORSPACE_NAME*/ 64] = "\0"; - MovieReader *anim = openanim(job_data->path, IB_byte_data, 0, colorspace); + MovieReader *anim = openanim(job_data->path, IB_byte_data, 0, true, colorspace); if (anim != nullptr) { g_drop_coords.strip_len = MOV_get_duration_frames(anim, IMB_TC_NONE); diff --git a/source/blender/imbuf/intern/thumbs.cc b/source/blender/imbuf/intern/thumbs.cc index 8c3247ee595..5ddefd64fc8 100644 --- a/source/blender/imbuf/intern/thumbs.cc +++ b/source/blender/imbuf/intern/thumbs.cc @@ -393,7 +393,10 @@ static ImBuf *thumb_create_ex(const char *file_path, } else if (THB_SOURCE_MOVIE == source) { MovieReader *anim = nullptr; - anim = MOV_open_file(file_path, IB_byte_data | IB_metadata, 0, nullptr); + /* Image buffer is converted from float to byte and only the latter one is used, and the + * conversion process is aware of the float colorspace. So it is possible to save some + * compute time by keeping the original colorspace for movies. */ + anim = MOV_open_file(file_path, IB_byte_data | IB_metadata, 0, true, nullptr); if (anim != nullptr) { img = MOV_decode_frame(anim, 0, IMB_TC_NONE, IMB_PROXY_NONE); if (img == nullptr) { diff --git a/source/blender/imbuf/movie/MOV_read.hh b/source/blender/imbuf/movie/MOV_read.hh index 1640b0bc77e..42c7dbf434d 100644 --- a/source/blender/imbuf/movie/MOV_read.hh +++ b/source/blender/imbuf/movie/MOV_read.hh @@ -38,6 +38,7 @@ struct MovieProxyBuilder; MovieReader *MOV_open_file(const char *filepath, int ib_flags, int streamindex, + bool keep_original_colorspace, char colorspace[IM_MAX_SPACE]); /** diff --git a/source/blender/imbuf/movie/intern/movie_proxy_indexer.cc b/source/blender/imbuf/movie/intern/movie_proxy_indexer.cc index 26cd33a2fbd..c3e669a8050 100644 --- a/source/blender/imbuf/movie/intern/movie_proxy_indexer.cc +++ b/source/blender/imbuf/movie/intern/movie_proxy_indexer.cc @@ -1258,8 +1258,11 @@ MovieReader *movie_open_proxy(MovieReader *anim, IMB_Proxy_Size preview_size) get_proxy_filepath(anim, preview_size, filepath, false); - /* proxies are generated in the same color space as animation itself */ - anim->proxy_anim[i] = MOV_open_file(filepath, 0, 0, anim->colorspace); + /* Proxies are generated in the same color space as animation itself. + * + * Also skip any colorspace conversion to the color pipeline design as it helps performance and + * the image buffers from the proxy builder are not used anywhere else in Blender. */ + anim->proxy_anim[i] = MOV_open_file(filepath, 0, 0, true, anim->colorspace); anim->proxies_tried |= preview_size; diff --git a/source/blender/imbuf/movie/intern/movie_read.cc b/source/blender/imbuf/movie/intern/movie_read.cc index 2302564fa31..5b2cc1a5ae9 100644 --- a/source/blender/imbuf/movie/intern/movie_read.cc +++ b/source/blender/imbuf/movie/intern/movie_read.cc @@ -99,8 +99,9 @@ IDProperty *MOV_load_metadata(MovieReader *anim) } MovieReader *MOV_open_file(const char *filepath, - int ib_flags, - int streamindex, + const int ib_flags, + const int streamindex, + const bool keep_original_colorspace, char colorspace[IM_MAX_SPACE]) { MovieReader *anim; @@ -122,6 +123,7 @@ MovieReader *MOV_open_file(const char *filepath, STRNCPY(anim->filepath, filepath); anim->ib_flags = ib_flags; anim->streamindex = streamindex; + anim->keep_original_colorspace = keep_original_colorspace; } return anim; } @@ -1248,11 +1250,9 @@ static ImBuf *ffmpeg_fetchibuf(MovieReader *anim, int position, IMB_Timecode_Typ MEM_mallocN_aligned(pixel_size * anim->x * anim->y, align, "ffmpeg ibuf")); if (anim->is_float) { IMB_assign_float_buffer(cur_frame_final, (float *)buffer_data, IB_TAKE_OWNERSHIP); - cur_frame_final->float_buffer.colorspace = colormanage_colorspace_get_named(anim->colorspace); } else { IMB_assign_byte_buffer(cur_frame_final, buffer_data, IB_TAKE_OWNERSHIP); - cur_frame_final->byte_buffer.colorspace = colormanage_colorspace_get_named(anim->colorspace); } AVFrame *final_frame = ffmpeg_frame_by_pts_get(anim, pts_to_search); @@ -1268,6 +1268,30 @@ static ImBuf *ffmpeg_fetchibuf(MovieReader *anim, int position, IMB_Timecode_Typ ffmpeg_postprocess(anim, final_frame, cur_frame_final); } + if (anim->is_float) { + if (anim->keep_original_colorspace) { + /* Movie has been explicitly requested to keep original colorspace, regardless of the nature + * of the buffer. */ + cur_frame_final->float_buffer.colorspace = colormanage_colorspace_get_named( + anim->colorspace); + } + else { + /* Float buffers are expected to be in the scene linear color space. + * Linearize the buffer if it is in a different space. + * + * It might not be the most optimal thing to do from the playback performance in the + * sequencer perspective, but it ensures that other areas in Blender do not run into obscure + * color space mismatches. */ + colormanage_imbuf_make_linear(cur_frame_final, anim->colorspace); + } + } + else { + /* Colorspace conversion is lossy for byte buffers, so only assign the colorspace. + * It is up to artists to ensure operations on byte buffers do not involve mixing different + * colorspaces. */ + cur_frame_final->byte_buffer.colorspace = colormanage_colorspace_get_named(anim->colorspace); + } + anim->cur_position = position; return cur_frame_final; diff --git a/source/blender/imbuf/movie/intern/movie_read.hh b/source/blender/imbuf/movie/intern/movie_read.hh index 1b8642ac545..7b3f863c7fd 100644 --- a/source/blender/imbuf/movie/intern/movie_read.hh +++ b/source/blender/imbuf/movie/intern/movie_read.hh @@ -48,6 +48,8 @@ struct MovieReader { int streamindex = 0; + bool keep_original_colorspace = false; + #ifdef WITH_FFMPEG AVFormatContext *pFormatCtx = nullptr; AVCodecContext *pCodecCtx = nullptr; diff --git a/source/blender/sequencer/intern/cache/thumbnail_cache.cc b/source/blender/sequencer/intern/cache/thumbnail_cache.cc index f6878219918..a9498ba0b50 100644 --- a/source/blender/sequencer/intern/cache/thumbnail_cache.cc +++ b/source/blender/sequencer/intern/cache/thumbnail_cache.cc @@ -359,7 +359,8 @@ void ThumbGenerationJob::run_fn(void *customdata, wmJobWorkerStatus *worker_stat cur_anim_path = request.file_path; cur_stream = request.stream_index; - cur_anim = MOV_open_file(cur_anim_path.c_str(), IB_byte_data, cur_stream, nullptr); + cur_anim = MOV_open_file( + cur_anim_path.c_str(), IB_byte_data, cur_stream, true, nullptr); } /* Decode the movie frame. */ diff --git a/source/blender/sequencer/intern/proxy.cc b/source/blender/sequencer/intern/proxy.cc index 81f02cedefe..918c68a069c 100644 --- a/source/blender/sequencer/intern/proxy.cc +++ b/source/blender/sequencer/intern/proxy.cc @@ -222,7 +222,10 @@ ImBuf *seq_proxy_fetch(const RenderData *context, Strip *strip, int timeline_fra return nullptr; } - proxy->anim = openanim(filepath, IB_byte_data, 0, strip->data->colorspace_settings.name); + /* Sequencer takes care of colorspace conversion of the result. The input is the best to be + * kept unchanged for the performance reasons. */ + proxy->anim = openanim( + filepath, IB_byte_data, 0, true, strip->data->colorspace_settings.name); } if (proxy->anim == nullptr) { return nullptr; diff --git a/source/blender/sequencer/intern/render.cc b/source/blender/sequencer/intern/render.cc index 6bc89423be2..5eb8f39870b 100644 --- a/source/blender/sequencer/intern/render.cc +++ b/source/blender/sequencer/intern/render.cc @@ -1030,7 +1030,10 @@ static ImBuf *seq_render_movie_strip_custom_file_proxy(const RenderData *context if (proxy->anim == nullptr) { if (seq_proxy_get_custom_file_filepath(strip, filepath, context->view_id)) { - proxy->anim = openanim(filepath, IB_byte_data, 0, strip->data->colorspace_settings.name); + /* Sequencer takes care of colorspace conversion of the result. The input is the best to be + * kept unchanged for the performance reasons. */ + proxy->anim = openanim( + filepath, IB_byte_data, 0, true, strip->data->colorspace_settings.name); } if (proxy->anim == nullptr) { return nullptr; diff --git a/source/blender/sequencer/intern/strip_add.cc b/source/blender/sequencer/intern/strip_add.cc index c4a091aa256..599749a51fb 100644 --- a/source/blender/sequencer/intern/strip_add.cc +++ b/source/blender/sequencer/intern/strip_add.cc @@ -419,7 +419,9 @@ Strip *add_movie_strip(Main *bmain, Scene *scene, ListBase *seqbase, LoadData *l char filepath_view[FILE_MAX]; seq_multiview_name(scene, i, prefix, ext, filepath_view, sizeof(filepath_view)); - anim_arr[j] = openanim(filepath_view, IB_byte_data, 0, colorspace); + /* Sequencer takes care of colorspace conversion of the result. The input is the best to be + * kept unchanged for the performance reasons. */ + anim_arr[j] = openanim(filepath_view, IB_byte_data, 0, true, colorspace); if (anim_arr[j]) { seq_anim_add_suffix(scene, anim_arr[j], i); @@ -431,7 +433,9 @@ Strip *add_movie_strip(Main *bmain, Scene *scene, ListBase *seqbase, LoadData *l } if (is_multiview_loaded == false) { - anim_arr[0] = openanim(filepath, IB_byte_data, 0, colorspace); + /* Sequencer takes care of colorspace conversion of the result. The input is the best to be + * kept unchanged for the performance reasons. */ + anim_arr[0] = openanim(filepath, IB_byte_data, 0, true, colorspace); } if (anim_arr[0] == nullptr && !load_data->allow_invalid_file) { @@ -586,9 +590,12 @@ void add_reload_new_file(Main *bmain, Scene *scene, Strip *strip, const bool loc char filepath_view[FILE_MAX]; seq_multiview_name(scene, i, prefix, ext, filepath_view, sizeof(filepath_view)); + /* Sequencer takes care of colorspace conversion of the result. The input is the best + * to be kept unchanged for the performance reasons. */ anim = openanim(filepath_view, IB_byte_data | ((strip->flag & SEQ_FILTERY) ? IB_animdeinterlace : 0), strip->streamindex, + true, strip->data->colorspace_settings.name); if (anim) { @@ -603,11 +610,14 @@ void add_reload_new_file(Main *bmain, Scene *scene, Strip *strip, const bool loc } if (is_multiview_loaded == false) { - MovieReader *anim; - anim = openanim(filepath, - IB_byte_data | ((strip->flag & SEQ_FILTERY) ? IB_animdeinterlace : 0), - strip->streamindex, - strip->data->colorspace_settings.name); + /* Sequencer takes care of colorspace conversion of the result. The input is the best to be + * kept unchanged for the performance reasons. */ + MovieReader *anim = openanim(filepath, + IB_byte_data | + ((strip->flag & SEQ_FILTERY) ? IB_animdeinterlace : 0), + strip->streamindex, + true, + strip->data->colorspace_settings.name); if (anim) { sanim = MEM_mallocN("Strip Anim"); BLI_addtail(&strip->anims, sanim); diff --git a/source/blender/sequencer/intern/utils.cc b/source/blender/sequencer/intern/utils.cc index 2924bbfe06d..d4ca81868c9 100644 --- a/source/blender/sequencer/intern/utils.cc +++ b/source/blender/sequencer/intern/utils.cc @@ -208,10 +208,13 @@ ListBase *get_seqbase_from_strip(Strip *strip, ListBase **r_channels, int *r_off static void open_anim_filepath(Strip *strip, StripAnim *sanim, const char *filepath, bool openfile) { + /* Sequencer takes care of colorspace conversion of the result. The input is the best to be + * kept unchanged for the performance reasons. */ if (openfile) { sanim->anim = openanim(filepath, IB_byte_data | ((strip->flag & SEQ_FILTERY) ? IB_animdeinterlace : 0), strip->streamindex, + true, strip->data->colorspace_settings.name); } else { @@ -219,6 +222,7 @@ static void open_anim_filepath(Strip *strip, StripAnim *sanim, const char *filep IB_byte_data | ((strip->flag & SEQ_FILTERY) ? IB_animdeinterlace : 0), strip->streamindex, + true, strip->data->colorspace_settings.name); } } diff --git a/source/blender/windowmanager/intern/wm_playanim.cc b/source/blender/windowmanager/intern/wm_playanim.cc index f348395e096..49b69ec0ff3 100644 --- a/source/blender/windowmanager/intern/wm_playanim.cc +++ b/source/blender/windowmanager/intern/wm_playanim.cc @@ -855,7 +855,7 @@ static void build_pict_list_from_anim(ListBase &picsbase, const int frame_offset) { /* OCIO_TODO: support different input color space. */ - MovieReader *anim = MOV_open_file(filepath_first, IB_byte_data, 0, nullptr); + MovieReader *anim = MOV_open_file(filepath_first, IB_byte_data, 0, false, nullptr); if (anim == nullptr) { CLOG_WARN(&LOG, "couldn't open anim '%s'", filepath_first); return; @@ -1823,7 +1823,9 @@ static std::optional wm_main_playanim_intern(int argc, const char **argv, P filepath = argv[0]; if (MOV_is_movie_file(filepath)) { /* OCIO_TODO: support different input color spaces. */ - MovieReader *anim = MOV_open_file(filepath, IB_byte_data, 0, nullptr); + /* Image buffer is used for display, which does support displaying any buffer from any + * colorspace. Skip colorspace conversions in the movie module to improve performance. */ + MovieReader *anim = MOV_open_file(filepath, IB_byte_data, 0, true, nullptr); if (anim) { ibuf = MOV_decode_frame(anim, 0, IMB_TC_NONE, IMB_PROXY_NONE); MOV_close(anim);