[FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

[Hendrik Leppkes]

On Thu, May 18, 2017 at 11:16 AM, Nicolas George <george at nsup.org> wrote:
> Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit :
>> I think its a saner choice to design the API to try to avoid instant
>> heap corruption, instead of hoping every case sets the alignment
>> properly in all cases.
>> A single if condition shouldn't be too much trouble, we already flag
>> hardware pixel formats as such in the pixdesc to identify them.
>
> Giving an incorrect alignment value to this function will not cause an
> "instant heap corruption", you are burning a straw man.

If as a result of this function returning 0 some other code tries to
memcpy some opaque hardware pointer, there is a good chance of
something crazy happening, so its hardly a strawman.
We could put the check in another central place, but IMHO it needs to
be somewhere central, and not hope that every filter and encoder
remembers to override alignment in the hardware surface input case.

>
> This function performs a simple task: it checks the alignment of a
> frame, given as a parameter, against a required alignment value, given
> as a parameter. Simple, straightforward, predictable.
>
> Adding special cases to this function would break these properties.

This simple task relies on various properties of the input frame -
which in the hardware case are just not given, so its essentially
performing an undefined operation.

>
> Also, I am roughly one bikeshedding mail away from losing interest in
> this whole matter.
>

This is not bikeshedding, its an actual concern.

- Hendrik