On 30/01/18 14:20, wm4 wrote:
> On Tue, 30 Jan 2018 14:09:43 +0000
> Mark Thompson <s...@jkqxz.net> wrote:
>
>> On 30/01/18 07:24, Muhammad Faiz wrote:
>>> Move REGISTER_FILTER to FILTER_TABLE in configure.
>>> Auto generate filter extern and filter table.
>>> Sort filter table, use bsearch on avfilter_get_by_name.
>>> Define next pointer at filter extern, no need to initialize
>>> next pointer at run time, so AVFilter can be set to const.
>>> Make avfilter_register always return error.
>>> Target checkasm now depends on EXTRALIBS-avformat.
>>>
>>> Signed-off-by: Muhammad Faiz <mfc...@gmail.com>
>>> ---
>>
>> I like the idea of this, but I'm not sure about some of the implementation
>> details.
>>
>> Have you considered dropping the "next" links entirely and having just the
>> array of pointers instead? I feel like they don't really add anything
>> useful once we are in this form, and result in more boilerplate on every
>> filter and some tricky generation code.
>>
>> avfilter_next() would be a bit slower (since it would have to search the
>> array, and therefore have runtime linear in the number of filters), but
>> avfilter_get_by_name() is a lot faster because of the binary search (and is
>> the only common use of it) so I don't think that complaint would be a strong
>> one.
>
> I think we considered that for libavcodec, but discarded the idea
> because ffmpeg.c's use was causing noticable performance problems? (I
> don't remember the details.) Also wouldn't the runtime be quadratic
> rather than linear when iterating filters?
Linear per-call, quadratic to iterate over all.
Do you have any reference for that discussion so we can compare? The only call
to avfilter_next() in ffmpeg.c is for showing the filter list (-filters), which
I don't think is a performance-sensitive option.
Also, avfilter_next() can be made faster (possibly, constants will be larger)
by using the binary search in the list since you know the name of the current
filter (it's in the pointer you are given).
>>> +const AVFilter *avfilter_next(const AVFilter *prev)
>>> +{
>>> + CHECK_VALIDITY();
>>
>> Calling avfilter_next() without having called avfilter_register_all()
>> violates the API, though?
>
> Before this change it just returned NULL, now it returns the full list.
> I think that was the intent and I don't think it's a problem, besides
> keeping the old behavior would require mutable state.
I was thinking of just not having the validity check there. Maybe it doesn't
matter.
>> (Or is there an intent to deprecate avfilter_register_all() immediately
>> after this?)
>
> Hopefully.
Sounds good :)
Thanks,
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
