On Fri, 21 Jul 2017 23:02:12 +0200
Anton Khirnov <[email protected]> wrote:

> Quoting wm4 (2017-07-21 22:56:43)
> > On Fri, 21 Jul 2017 22:32:43 +0200
> > Anton Khirnov <[email protected]> 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.)  
> 
> Then we should document those functions as allowing unaligned data and
> then what I wrote will still be true. And for sws i would almost expect
> a copy+simd to be faster than the C paths.
> 

Right.

Anyway, will send an updated patch with apichanges + version bumps some
time.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to