Re: [FFmpeg-devel] [PATCH v2 1/1] avfilter/vf_vpp_qsv: apply 3D LUT from file.

[Zhao Zhili <quinkblack@foxmail.com>]

> On Jan 28, 2024, at 22:10, Anton Khirnov <anton@khirnov.net> wrote:
> 
> Quoting Zhao Zhili (2024-01-28 14:51:58)
>> 
>> 
>>> On Jan 28, 2024, at 18:31, Anton Khirnov <anton@khirnov.net> wrote:
>>> 
>>> Quoting Chen Yufei (2024-01-25 17:16:46)
>>>> On Wed, Jan 24, 2024 at 7:39 PM Anton Khirnov <anton@khirnov.net> wrote:
>>>>> 
>>>>> Quoting Chen Yufei (2024-01-20 16:14:29)
>>>>>> Usage: "vpp_qsv=lut3d_file=<path to file>"
>>>>> 
>>>>> Passing file paths to a filter and having the filter load the file is
>>>>> not recommended, it is generally preferable to have an
>>>>> AV_OPT_TYPE_BINARY option, with IO performed by the caller.
>>>>> 
>>>>> E.g. in ffmpeg CLI you can do
>>>>> -filter vpp_qsv=/lut3d=<file>
>>>>> to load the option value from a file.
>>>> 
>>>> I searched for code using `AV_OPT_TYPE_BINARY`.
>>>> `vf_libplacebo.c` gives me a good example.
>>>> 
>>>> The LUT parsing code is took from `libavfilter/vf_lut3d.c`.
>>>> It's mainly text processing which calls functions on `FILE*`.
>>>> Using `AV_OPT_TYPE_BINARY` would require many changes in LUT paring
>>>> code, and also need to change the command line option of `vf_lut3d`.
>>>> So I'd keep the lut file option as is.
>>> 
>>> Your patch is rejected then.
>> 
>> Could you be more elaborate?
> 
> I said in my previous email what I believe should be done instead. I'm
> open to reasonable arguments about that of course, but "I am going to
> ignore your review" is not a good way to get patches in.
> 
>> I agree pass LUT as file path isn’t flexible and clean, but it’s more questionable to
>> use AV_OPT_TYPE_BINARY for data which could be a large chunk.
> 
> The LUT is loaded into memory in any case, is it not?

The binary to string in hex format to binary is like transport image in base64. It doesn’t
sound right to use base64 for video streams.

> 
>> Pass as file path works for me for now since it’s not the first case. We can replace
>> it if anyone has a better idea.
> 
> Every new patch like this is just more work somebody has to undo in the
> future. Given past experience, I'd MUCH prefer to do it properly in the
> first place.
> 
> -- 
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto: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".