On Sun, Jan 28, 2024 at 10:10 PM Anton Khirnov <an...@khirnov.net> wrote: > > Quoting Zhao Zhili (2024-01-28 14:51:58) > > > > > > > On Jan 28, 2024, at 18:31, Anton Khirnov <an...@khirnov.net> wrote: > > > > > > Quoting Chen Yufei (2024-01-25 17:16:46) > > >> On Wed, Jan 24, 2024 at 7:39 PM Anton Khirnov <an...@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.
"is not recommended, it is generally preferable" I take this as using `AV_OPT_TYPE_BINARY` is not a MUST. I'm not an English native speaker, correct me If I'm wrong. > > >>> > > >>> 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. Now I understand using `AV_OPT_TYPE_BINARY` is a must. > > > > 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. > The review is not ignored. I've taken time to search for how to use `AV_OPT_TYPE_BINARY` and did not think it's appropriate to use in this case. If someone can show me code in FFmpeg that parses loaded data as text, I can follow that and send a new patch. This is to avoid doing things the wrong way, as I'm a newcomer and don't know if there's existing code to do this. > > 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? > > > 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 -- Best regards, Chen Yufei _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
