On Tue, Jan 30, 2024 at 1:07 AM Anton Khirnov <an...@khirnov.net> wrote: > > Quoting Chen Yufei (2024-01-29 04:01:51) > > 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. > > Okay, sorry for being unclear. When I say 'recommended', it means it > must be done this way unless there is a strong technical reason to do it > otherwise. But then please provide a detailed explanation, not > something vague like "didn't seem appropriate".
After taking more time looking into how to use `AV_OPT_TYPE_BINARY`, I'll give more details. 1. For LUT file parsing, I'm not writing new code, it's taken from existing vf_lut3d.c. I avoid making unnecessary modifications to avoid breaking things. 2. The original code relies on file name to detect LUT file type, uses `fgets` and the like. From my understanding, `AV_OPT_TYPE_BINARY` does not give the file name to the calling filter. If we use `AV_OPT_TYPE_BINARY`, then how to do file type detection? Adding other cmdline option would require changing vf_lut3d's cmdline option. 3. I tried to find basic text processing utils in FFmpeg to avoid writing ad-hoc text processing code in lut3d.c I've found `FFTextReader` in libavformat/subtitles.h, but I don't think I should use it in filter code. I'm not sure if it's OK to follow the code in `FFTextReader` and duplicate read line logic in lut3d.c. Besides, there's `BINARY` in the name of `AV_OPT_TYPE_BINARY`, but what I'm specifying here is text file. This is not a big problem, but seem like an option name like `AV_OPT_TYPE_TEXT` can express the usage of the option more clear in code. > > -- > 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".