[FFmpeg-devel] [PATCH v4 3/7] cmdutils: use new iteration APIs

[Josh de Kock]

On 2018/03/23 10:05, Nicolas George wrote:
> Josh de Kock (2018-03-22):
>> move lavd avinputformats and avoutputformats into lavf
>>
>> delete lavd
> 
> Possibly ok in principle for me, but see below.
> 

I personally think this will fix a lot of the weird interactions between 
the two libraries, and should be considered a serious option independent 
of the new API.

>> write new lavd aimed at actual devices
> 
> There are already such libraries, we do not need another. The basic
> devices with a (de)muxer API are quite right to give many extra features
> with little extra cost.

After thinking about this for a while, I actually agree somewhat.

> But why are we discussing this? It seems to me that the discussion went
> approximately like this:
> 
> "Darn, the faucet I just bought to fix the leaky one does not fit the
> pipes. Well, I guess I will have to redo the whole plumbing to make it
> fit."

Sure, I guess but I still think it's more involved than just 'one faucet 
not fitting'. There's still an entire pipe which doesn't fit but was 
still installed with duct tape.

> The correct way of addressing the problem is to buy a new faucet with
> the correct size. And cut the losses if the first one cannot be
> refunded. I feel like the discussion is largely fueled by the cognitive
> bias known as "sunk cost fallacy": due to efforts invested in a
> solution, become attached emotionally to it and fail to see when it
> proves to cause more costs than benefits.

I mean it's so far gone that I don't think it matters how long it takes 
anymore as long as it gets done, and gets done 'right'. This is a 
release blocker, and yes that's my bad but I do think that some earlier 
help (when I asked even before starting the new API) from others would 
have maybe avoided the current situation.

> Can we at least REALLY CONSIDER this:
> 
> 1. Acknowledge that this issue about lavd, on top of Michael's early
>     concerns about registering external components, has proven that the
>     all-static approach, while elegant in many ways, is not practical.
> 
> 2. Agree to revert the API as it is and discuss a better solution.

If you submit a set to revert it (my git skills suck), I won't block it, 
provided there will actually be discussion--from what I've seen, 
discussion only occurs after things happen, which isn't very helpful for 
larger more impactful changes.

> 3. As for the better solution, I really propose to register the
>     components (with av_register_something calls, like now) into an array
>     rather than a linked list (like your proposal) that is stored in a
>     user-provided structure that can exist in several instances (new),
>     with a global instance of that structure for temporary compatibility.

So the reason I liked having an iteration-style function was that it 
allowed an API user to get an array of all the components *but it didn't 
force them*.

Your suggestion here is interesting, from what I understand it seems to 
be a thread-local, user-managed state of the loaded components in each 
library? Coupled with a new and proper registration API (the previous 
one wasn't ideal for registering external components. The downside of 
this approach is then you go in completely the opposite direction: the 
library requires even more 'bootstrap'. The reason for the new API 
wasn't just to use arrays internally, but to have no initialisation 
required.

> Note that with this proposal, your efforts are not wasted: most of the
> code can be reused, and they have taught us a valuable lesson on top of
> that.

I hope that in the future it will be less of a 'send a patch and we'll 
ACK/NACK', because some things really benefit from discussion.

-- 
Josh