> -----Original Message----- > From: ffmpeg-devel <[email protected]> On Behalf Of Anton > Khirnov > Sent: 2024年1月29日 0:52 > To: FFmpeg development discussions and patches <[email protected]> > Cc: Haihao Xiang <[email protected]>; Chen Yufei <[email protected]> > Subject: Re: [FFmpeg-devel] [PATCH v2 1/1] avfilter/vf_vpp_qsv: apply 3D LUT > from file. > > Quoting Zhao Zhili (2024-01-28 16:46:55) > > > > > > > On Jan 28, 2024, at 22:10, Anton Khirnov <[email protected]> wrote: > > > > > > Quoting Zhao Zhili (2024-01-28 14:51:58) > > >> > > >> > > >>> On Jan 28, 2024, at 18:31, Anton Khirnov <[email protected]> wrote: > > >>> > > >>> Quoting Chen Yufei (2024-01-25 17:16:46) > > >>>> On Wed, Jan 24, 2024 at 7:39 PM Anton Khirnov <[email protected]> > > >>>> 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. > > I don't quite see the connection. Where is anything converted to a hex > string?
When the option value is set via av_opt_set instead of av_opt_set_bin. av_opt_set_bin can avoid such inefficiency, but it still do memcpy. > > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > [email protected] > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > [email protected] with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
