[FFmpeg-devel] [PATCH] avformat/utils: add helper functions to retrieve index entries from an AVStream

[Nicolas George]

James Almer (12021-03-23):
> I recall many people have said before that just because it was done before
> doesn't mean it should be done again. That's how bad practices spread.

I will happily concede you this. All the more happily that I will keep
it warm to serve it back when you oppose to one of my creative API
inventions for the sake of consistency ;-Þ

> > 	AVStream **streams = ctx->streams;
> > 	av_read_frame(ctx, &packet);
> > 	AVStream *stream = streams[packet.stream_index];
> No, avformat_new_stream() will reallocate that array, so if av_read_frame()
> can allocate new streams (I think AVFMT_NOHEADER formats do that) then that
> may just crash.

This is exactly why I chose this example.

> You should always use ctx->streams directly.

I am using ctx->streams directly. More accurately, we should always use
ctx->streams *immediately*.

And that's exactly the same with giving access to the internal
structure: they use the fields *immediately*, and everything is fine.

What you are defending is equivalent to defending
avformat_get_stream(ctx, idx) just to prevent users from accessing
ctx->streams directly because they may keep a pointer to an outdated
array. Note that in my proposal, the constraint is clearly documented.
This is not the case, currently, for ctx->streams.

Furthermore, returning a pointer avoids copying the fields that the user
will not use. It is minuscule, of course. But it is still one more
argument for returning the pointer.

We have to choose between having an API proof against users who do not
read the doc and try to guess what they can get away with and having an
API that is efficient and convenient for careful users, we cannot have
both.

I vote we choose the second, because the first is not really possible.

Regards,

-- 
  Nicolas George