On Fri, 21 Jul 2017 22:32:43 +0200
Anton Khirnov <an...@khirnov.net> wrote:
> Quoting wm4 (2017-07-12 17:02:49)
> > +int av_frame_apply_cropping(AVFrame *frame, int flags)
> > +{
> > + const AVPixFmtDescriptor *desc;
> > + size_t offsets[4];
> > + int i;
> > +
> > + if (!(frame->width > 0 && frame->height > 0))
> > + return AVERROR(EINVAL);
> > +
> > + /* make sure we are noisy about decoders returning invalid cropping
> > data */
>
> The comment does not apply anymore.
>
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index f9ffb5bbbf..eb1fa0cba5 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -580,6 +580,38 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame
> > *frame,
> > */
> > void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType
> > type);
> >
> > +
> > +/**
> > + * Flags for frame cropping.
> > + */
> > +enum {
> > + /**
> > + * Apply the maximum possible cropping, even if it requires setting the
> > + * AVFrame.data[] entries to unaligned pointers. Using this is strongly
> > + * discouraged, especially if the data is to be fed back to Libav API.
> > It
> > + * can cause performance issues, and in some cases crashes.
>
> 'discouraged' is too weak IMO, to me it means that it's something which
> is allowed but suboptimal. Whereas passing invalid data to libav* is
> outright forbidden as UB, and is very likely to crash. Suggested
> wording:
> 'Note that all Libav APIs (except those that explicitly specify
> otherwise) expect the frame data to be aligned, so when this flag is
> used, the resulting frame must not be fed back to Libav.'
>
> Otherwise looks ok, modulo apichanges/bump
>
Isn't that wording to strict? I'm fairly sure you'd still be allowed to
e.g. copy the frame data, or pass it to swscale. (Yes, libswscale is
officially fine with unaligned data, just that it might switch to C
code paths.)
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
