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

[Nicolas George]

Josh de Kock (2018-03-23):
> 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.

Maybe. But that is for another discussion.

> 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.

Possibly, but that still does not warrant redoing the whole plumbing.

> 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,

This is one of the reasons I am against releases. Still, we can revert
and take all the time we need.

>	   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.

> 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.

I completely agree that this project is awful in that matter. We should
strive to change that and find better practices.

Maybe post a mail tagged [METAPATCH] to discuss the principle of the
changes, treat it like a patch in that it needs to be approved before
being adopted. But once it is adopted, the discussion can move forward
with the patches themselves, and objections about the principle can be
disregarded.

Note that this would not have helped much here, since nobody foresaw the
complication about lavd.

> 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.

I will try to do just that, if time permits. But time is very short for
me this week.

> 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*.

If an array is available, you can always look at only one cell.

> 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.

Some of your comments show that I did not explain what I propose well
enough, causing you to completely misunderstand it. Let me explain it
more precisely. There is nothing thread-local about it. What I propose
looks like this:

	AVFormatLibrary *lavf = avformat_library_alloc();
	avformat_register_all(lavf);
	avdevice_register_all(lavf);
	...
	avformat_open_input2(lavf, &ctx, url, NULL, &options);

The benefit is that the global state becomes explicit, and not global:
it can exist in several instances.

Another benefit is that is provides a much cleaner and more reliable
black-/white-listing solution that the one we currently have.

I acknowledge your efforts to make things more static, but I think this
discussion has proven it was a mistake. We cannot eradicate all needs
for dynamic initialization, and a design that is meant for almost all
static will make the few cases where static is not possible very
hackish.

You can observe just that by the fact that you needed to add an avpriv
function to let lavd communicate with lavf. Each time an avpriv function
appears, it shows there is something flawed in the design.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180325/ecf07dbc/attachment.sig>