[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

Juan De León juandl at google.com
Wed Aug 14 20:43:34 EEST 2019


On Wed, Aug 14, 2019 at 12:10 AM Nicolas George <george at nsup.org> wrote:

> James Almer (12019-08-13):
> > I'm fairly sure this was discussed before, but invalid arguments
> > shouldn't crash an user's application. They even have their own
> > standardized errno value for this purpose.
> > asserts() are to catch bugs in our code, not in theirs. Returning a NULL
> > pointer is the preferred behavior.
>
> I do not agree. Asserts are to catch all bugs that can be catched
> statically. There is no sense in making some bugs harder to catch just
> because they involve separate developers.
>
> Nobody can predict whether a disk will make a I/O error, whether there
> will be enough memory, etc., that kind of error MUST be handled
> gracefully.
>
> On the other hand, it is easy to make sure that a buffer given to read()
> is not NULL, and therefore it is very acceptable to just crash when that
> happens.
>
> EINVAL is for cases where the acceptable value cannot be easily
> predicted by the caller. For example, setting an unsupported sample rate
> for the sound device: the caller could check in advance, but that would
> be cumbersome.
>
> Now, please tell me, according to you, "idx is not smaller than
> nb_blocks", is it more akin to a disk I/O error, a NULL buffer or an
> invalid sample rate?
>
> Another argument:
>
> Instead of providing utility code, we could just document the
> arithmetic. In that case, the application would have code that says, in
> essence: "block = info + offset + idx * block_size". No check. What
> would happen if idx was too big? Not a graceful error: a crash, or
> worse.
>
> The assert mimics that, in a more convenient way since it gives the
> reason and line number, and does not allow the bug to devolve into
> something more serious than a crash.
>
> Regards,
>
> --
>   Nicolas George
> _______________________________________________
> 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".


In that case, I believe documenting the size of the array and behaviour of
undefined indexes should be enough. Have the pointers return NULL,
and let the user handle the result, instead of stopping the execution.

I would prefer to discuss the actual data structure for now.


More information about the ffmpeg-devel mailing list