[FFmpeg-devel] [PATCH 03/12] lavfi: drop vf_qp

[Thilo Borgmann]

Am 25.02.20 um 11:28 schrieb Anton Khirnov:
> Quoting Thilo Borgmann (2020-02-24 23:07:48)
>> Am 24.02.20 um 22:41 schrieb Lou Logan:
>>> On Mon, Feb 24, 2020, at 3:37 AM, Anton Khirnov wrote:
>>>> It fundamentally depends on an API that has been deprecated for five
>>>> years, has seen no commits since that time and is of highly dubious
>>>> usefulness.
>>>> ---
>>>>  doc/filters.texi            |  32 -------
>>>>  libavfilter/Makefile        |   1 -
>>>>  libavfilter/allfilters.c    |   1 -
>>>>  libavfilter/vf_qp.c         | 183 ------------------------------------
>>>>  tests/fate/filter-video.mak |   7 +-
>>>>  tests/ref/fate/filter-pp2   |   1 -
>>>>  tests/ref/fate/filter-pp3   |   1 -
>>>>  7 files changed, 1 insertion(+), 225 deletions(-)
>>>>  delete mode 100644 libavfilter/vf_qp.c
>>>>  delete mode 100644 tests/ref/fate/filter-pp2
>>>>  delete mode 100644 tests/ref/fate/filter-pp3
>>>
>>> Fine with me. I've never seen it used by anyone.
>>
>> I'm not fine with it. Declaring it's {use | use case} not existent is
>> no arguments whatsoever in reality.
>>
>> Also, removing some functionality needs an argument - it is not
>> keeping some functionality needs an argument.
> 
> I disagree with that. Keeping code around is not free, as Vittorio
> already said - it is a burden in many ways. So I believe all code needs
> continued justification for its existence - not just "it was added in
> the past so it stays in forever". Note that I'm not saying it needs to
> be mainstream or very popular - I am fine with obscure features that
> are only useful to a few people in highly special cases (given they are
> not unduly intrusive and those people are willing to maintain them). But
> so far in this thread, there has been no actual use presented for
> exporting and passing around QP tables. None whatsoever.
> 
> Most objections here are like yours - it's a) a feature and b) someone
> somewhere sometime might conceivably want to use it, so it must not be
> removed. Michael's reponse is the only one that comes close to having a
> use case, but even he says that he's unsure of the usefulness of the
> actual QP tables and that it's largely theoretical.
> 
> I believe I did more structural changes to the libraries than most
> people and in my experience these obsolete features from mplayer days
> are a MASSIVE maintenance pain. The amount of effort required to keep
> them working is simply not justified when essentially nobody uses them.
> 
> IMO these demands that they all be preserved forever is endangering the
> project. If making changes becomes too hard, people stop making them.
> They move to other places that are less hostile to change. We are at
> risk of turning into a repository of obscure old codecs and filters and
> getting overtaken by leaner projects like dav1d (yes it's supposed to be
> AV1-only, but that can change).
> 
>>
>> Nobody technically elaborates Paul's statement that it should go into side data. WTF? The compromise isn't even considered?
>>
>> Let's dig some trenches, shall we?
>>
>> And how come some obvious "use cases" / "needs" like [1] come into play? Or do we declare not continued discussions non-existent now, too?
> 
> The patch in your link is not using this API. Precisely because this API
> is inadequate for anything newer than MPEG4 ASP. If anything, it's an
> additional argument in favor of my patches.

My actual point about that patch was that there is a use case to extract QP tables for more current codecs. Suggests this use case is also there for less current ones which says we should not just remove this possibility.

If that patch is avoiding the deprecated things, could it be applied to the older codes and supersede vf_qp? (And I have no clue whether or not)
And would that not go into the direction Paul suggested?


>> And how comes, if Michael's investigation, that all of this is based on use of _a function_ that is deprecated instead of direct access of AVFrame's fields is the cause of all of this?
>>
>> Shame on all of us.
> 
> WTF? What shame?

I am ashamed that I do read most arguments here as merely black-and-white statements.
That might be considered, as I admit, as an overreaction and I probably should not have written.
However, that is also why I suggested to dig trenches.


> I am sending patches, in good faith, that I believe will improve the
> project. People may (and do) object to them. As long as the discussion
> is civil, constructive and in good faith (so far it mostly is), I see no
> reason for any shame.

Yes and please continue to do so. That I did not criticize.
Also yes, I don't see it non-civil and hopefully it will stay that way.
I see it, however, way less constructive as it could and should be.

-Thilo