From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 812B250314 for ; Fri, 11 Jul 2025 10:39:23 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 9592B68F158; Fri, 11 Jul 2025 13:39:19 +0300 (EEST) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id F2B5068ED81 for ; Fri, 11 Jul 2025 13:39:11 +0300 (EEST) Received: from haasn.dev (unknown [10.30.1.1]) by haasn.dev (Postfix) with UTF8SMTP id ACC09423A7; Fri, 11 Jul 2025 12:39:11 +0200 (CEST) Date: Fri, 11 Jul 2025 12:39:11 +0200 Message-ID: <20250711123911.GB5499@haasn.xyz> From: Niklas Haas To: ffmpeg-devel@ffmpeg.org In-Reply-To: <20250710151349.1157547-1-dev@lynne.ee> References: <20250710151349.1157547-1-dev@lynne.ee> MIME-Version: 1.0 Content-Disposition: inline Subject: Re: [FFmpeg-devel] [PATCH 1/7] vf_libplacebo: add support for specifying a LUT for the input X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Lynne Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Fri, 11 Jul 2025 00:13:29 +0900 Lynne wrote: > This makes it possible to apply Adobe .cube files to inputs. > --- > libavfilter/vf_libplacebo.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c > index 475030c80d..d74a6e1bf7 100644 > --- a/libavfilter/vf_libplacebo.c > +++ b/libavfilter/vf_libplacebo.c > @@ -159,6 +159,7 @@ typedef struct LibplaceboContext { > pl_vulkan vulkan; > pl_gpu gpu; > pl_tex tex[4]; > + struct pl_custom_lut *lut; > > /* input state */ > LibplaceboInput *inputs; > @@ -184,6 +185,7 @@ typedef struct LibplaceboContext { > AVExpr *pos_x_pexpr, *pos_y_pexpr, *pos_w_pexpr, *pos_h_pexpr; > float pad_crop_ratio; > float corner_rounding; > + char *lut_filename; > int force_original_aspect_ratio; > int force_divisible_by; > int reset_sar; > @@ -371,6 +373,28 @@ static int find_scaler(AVFilterContext *avctx, > return AVERROR(EINVAL); > } > > +static int update_lut(LibplaceboContext *s) > +{ > + int ret; > + uint8_t *lutbuf; > + size_t lutbuf_size; > + > + pl_lut_free(&s->lut); > + > + if ((ret = av_file_map(s->lut_filename, &lutbuf, &lutbuf_size, 0, s)) < 0) { > + av_log(s, AV_LOG_ERROR, > + "The LUT file '%s' could not be read: %s\n", > + s->lut_filename, av_err2str(ret)); > + return ret; > + } > + > + s->lut = pl_lut_parse_cube(s->log, lutbuf, lutbuf_size); It would be a good idea to at least warn or ideally error out if parsing the file fails (in which case, this function returns NULL). > + > + av_file_unmap(lutbuf, lutbuf_size); > + > + return 0; > +} > + > static int update_settings(AVFilterContext *ctx) > { > int err = 0; > @@ -468,6 +492,7 @@ static int update_settings(AVFilterContext *ctx) > RET(find_scaler(ctx, &opts->params.upscaler, s->upscaler, 0)); > RET(find_scaler(ctx, &opts->params.downscaler, s->downscaler, 0)); > RET(find_scaler(ctx, &opts->params.frame_mixer, s->frame_mixer, 1)); > + RET(update_lut(s)); Putting this function here will cause it to get called every time any of the settings change, which could in principle be very frequent. I would either embed a lightweight check to see if the filename matches the currently loaded filename, or else move this to the init() section and change it from DYNAMIC to STATIC, like how the custom shaders are handled. > > #if PL_API_VER >= 309 > while ((e = av_dict_get(s->extra_opts, "", e, AV_DICT_IGNORE_SUFFIX))) { > @@ -757,6 +782,7 @@ static void libplacebo_uninit(AVFilterContext *avctx) > av_freep(&s->inputs); > } > > + pl_lut_free(&s->lut); > #if PL_API_VER >= 351 > pl_cache_destroy(&s->cache); > #endif > @@ -1005,6 +1031,7 @@ static bool map_frame(pl_gpu gpu, pl_tex *tex, > .tex = tex, > .map_dovi = s->apply_dovi, > )); > + out->lut = s->lut; > > if (!s->apply_filmgrain) > out->film_grain.type = PL_FILM_GRAIN_NONE; > @@ -1406,6 +1433,7 @@ static const AVOption libplacebo_options[] = { > { "pad_crop_ratio", "ratio between padding and cropping when normalizing SAR (0=pad, 1=crop)", OFFSET(pad_crop_ratio), AV_OPT_TYPE_FLOAT, {.dbl=0.0}, 0.0, 1.0, DYNAMIC }, > { "fillcolor", "Background fill color", OFFSET(fillcolor), AV_OPT_TYPE_COLOR, {.str = "black@0"}, .flags = DYNAMIC }, > { "corner_rounding", "Corner rounding radius", OFFSET(corner_rounding), AV_OPT_TYPE_FLOAT, {.dbl = 0.0}, 0.0, 1.0, .flags = DYNAMIC }, > + { "lut", "Apply a look-up table", OFFSET(lut_filename), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = DYNAMIC }, Nit: Change it to "Path to custom LUT file to apply", since it takes a filename New option is missing an entry in doc/filters.texi > { "extra_opts", "Pass extra libplacebo-specific options using a :-separated list of key=value pairs", OFFSET(extra_opts), AV_OPT_TYPE_DICT, .flags = DYNAMIC }, > #if PL_API_VER >= 351 > { "shader_cache", "Set shader cache path", OFFSET(shader_cache), AV_OPT_TYPE_STRING, {.str = NULL}, .flags = STATIC }, > -- > 2.49.0.395.g12beb8f557c > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".