Re: [FFmpeg-devel] [PATCH 1/7] vf_libplacebo: add support for specifying a LUT for the input

[Lynne <dev@lynne.ee>]

On 11/07/2025 19:39, Niklas Haas wrote:
> On Fri, 11 Jul 2025 00:13:29 +0900 Lynne <dev@lynne.ee> 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).

Doesn't pl_lut_parse_cube do that already (its given a log context)?

>> +
>> +    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.

Is parsing really that heavy?
_______________________________________________
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".