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

[James Almer]

On 3/23/2021 4:07 PM, Nicolas George wrote:
> James Almer (12021-03-23):
>> I really don't like the idea of returning a pointer to some offset within an
>> internal struct that may either start pointing at some other entry or even
>> to freed memory, especially when the alternative of giving the user a copy
>> of a single entry is trivial to do and actually safe.
> 
> It is already what the users who accessed the field directly did.

Users accessed st->index_entries, or fields like 
st->index_entries[idx].timestamp. If the array was reallocated by lavf, 
st->index_entries is made to point to the new array, and accessing it or 
fields at offsets within it still worked as expected and gave you the 
information related to the index you requested.

If a function returns a pointer to some offset within an internal array, 
and said array changes or disappears, the pointer returned is now 
pointing to who knows what, and that's unsafe. A copy is trivial and safe.

> 
> And I am pretty sure I can find examples where we do similar things in
> the code base.

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.

> 
> And it is exactly what we are doing when we let users access fields
> directly.

See above. st->index_entries will always point to the current valid 
array, even if it was reallocated.

> 
> And the problem of sizeof(AVIndexEntry) part of the ABI just disappears
> if we do that.

It also disappears if we return a AVIndexEntry the user must free.

> 
> May I suggest that you recalibrate your dislike?

No, I'll not stop disliking this practice, for the reasons i stated 
above. And you'll not find me asking you to recalibrate you like, only 
explain why i think this new API should not use that approach.

> 
> Regards,
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>