Quoting wm4 (2017-01-10 10:26:53)
> On Tue, 10 Jan 2017 09:01:12 +0100
> Anton Khirnov <[email protected]> wrote:
> 
> > The current condition can trigger in cases where it shouldn't, with
> > unexpected results.
> > Make sure that:
> > - container cropping is really based on the original dimensions from the
> >   caller
> > - those dimenions are discarded on size change
> > 
> > The code is still quite hacky and eventually should be deprecated and
> > removed, with the decision about which cropping is used delegated to the
> > caller.
> > ---
> >  libavcodec/h264_slice.c | 15 +++++++++++----
> >  libavcodec/h264dec.c    |  3 +++
> >  libavcodec/h264dec.h    |  5 +++++
> >  3 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index 1b35c2b..a54d381 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -378,6 +378,8 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
> >      h->avctx->coded_width   = h1->avctx->coded_width;
> >      h->avctx->width         = h1->avctx->width;
> >      h->avctx->height        = h1->avctx->height;
> > +    h->width_from_caller    = h1->width_from_caller;
> > +    h->height_from_caller   = h1->height_from_caller;
> >      h->coded_picture_number = h1->coded_picture_number;
> >      h->first_field          = h1->first_field;
> >      h->picture_structure    = h1->picture_structure;
> > @@ -797,10 +799,15 @@ static int init_dimensions(H264Context *h)
> >      int height = h->height - (sps->crop_top   + sps->crop_bottom);
> >  
> >      /* handle container cropping */
> > -    if (FFALIGN(h->avctx->width,  16) == FFALIGN(width,  16) &&
> > -        FFALIGN(h->avctx->height, 16) == FFALIGN(height, 16)) {
> > -        width  = h->avctx->width;
> > -        height = h->avctx->height;
> > +    if (h->width_from_caller > 0 && h->height_from_caller > 0     &&
> > +        !sps->crop_top && !sps->crop_left                         &&
> > +        FFALIGN(h->width_from_caller,  16) == FFALIGN(width,  16) &&
> > +        FFALIGN(h->height_from_caller, 16) == FFALIGN(height, 16)) {
> > +        width  = h->width_from_caller;
> > +        height = h->height_from_caller;
> > +    } else {
> > +        h->width_from_caller  = 0;
> > +        h->height_from_caller = 0;
> >      }
> >  
> >      h->avctx->coded_width  = h->width;
> > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> > index e111d40..3209c1d 100644
> > --- a/libavcodec/h264dec.c
> > +++ b/libavcodec/h264dec.c
> > @@ -285,6 +285,9 @@ static int h264_init_context(AVCodecContext *avctx, 
> > H264Context *h)
> >  
> >      h->avctx                 = avctx;
> >  
> > +    h->width_from_caller     = avctx->width;
> > +    h->height_from_caller    = avctx->height;
> > +
> >      h->picture_structure     = PICT_FRAME;
> >      h->workaround_bugs       = avctx->workaround_bugs;
> >      h->flags                 = avctx->flags;
> > diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
> > index 5957e79..0a9896a 100644
> > --- a/libavcodec/h264dec.h
> > +++ b/libavcodec/h264dec.h
> > @@ -514,6 +514,11 @@ typedef struct H264Context {
> >       * the slice data */
> >      int field_started;
> >  
> > +    /* original AVCodecContext dimensions, used to handle container
> > +     * cropping */
> > +    int width_from_caller;
> > +    int height_from_caller;
> > +
> >      AVFrame *output_frame;
> >  
> >      int enable_er;
> 
> Is this kind of cropping even commonly used? Which containers?

mp4? I think it's quite common, or at least not very rare. We even have
a test for it. Ideally I'd prefer not handling it in lavc at all, but
since it's been grandfathered in we have to maintain it (for now at
least).

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to