Quoting Vittorio Giovara (2017-01-03 11:43:22)
> On Tue, Dec 27, 2016 at 7:31 PM, Anton Khirnov <an...@khirnov.net> wrote:
> > Also, add generic code for handling cropping, so the decoders can export
> > just the cropping size and not bother with the rest.
> > ---
> > doc/APIchanges | 4 ++
> > libavcodec/avcodec.h | 22 +++++++++
> > libavcodec/decode.c | 112
> > ++++++++++++++++++++++++++++++++++++++++++++-
> > libavcodec/internal.h | 6 +++
> > libavcodec/options_table.h | 1 +
> > libavcodec/version.h | 4 +-
> > 6 files changed, 146 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 10a2da4..a0ef198 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -13,6 +13,10 @@ libavutil: 2015-08-28
> >
> > API changes, most recent first:
> >
> > +2016-xx-xx - xxxxxxx - lavc 57.31.0 - avcodec.h
> > + Add AVCodecContext.apply_cropping to control whether cropping
> > + is handled by libavcodec or the caller.
> > +
> > 2016-xx-xx - xxxxxxx - lavu 55.30.0 - frame.h
> > Add AVFrame.crop_left/right/top/bottom fields for attaching cropping
> > information to video frames.
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 95da50b..524e06a 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -3112,6 +3112,28 @@ typedef struct AVCodecContext {
> > * This field should be set before avcodec_open2() is
> > called.
> > */
> > AVBufferRef *hw_frames_ctx;
> > +
> > + /**
> > + * Video decoding only. Certain video codecs support cropping, meaning
> > that
> > + * only a sub-rectangle of the decoded frame is intended for display.
> > This
> > + * option controls how cropping is handled by libavcodec.
> > + *
> > + * When set to 1 (the default), libavcodec will apply cropping
> > internally.
> > + * I.e. it will modify the output frame width/height fields and offset
> > the
> > + * data pointers (only by as much as possible while preserving
> > alignment, or
> > + * by the full amount if the AV_CODEC_FLAG_UNALIGNED flag is set) so
> > that
> > + * the frames output by the decoder refer only to the cropped area. The
> > + * crop_* fields of the output frames will be zero.
> > + *
> > + * When set to 0, the width/height fields of the output frames will be
> > set
> > + * to the coded dimensions and the crop_* fields will describe the
> > cropping
> > + * rectangle. Applying the cropping is left to the caller.
> > + *
> > + * When hardware acceleration with opaque output frames is used, the
> > actual
> > + * value of this option is disregarded and libavcodec behaves as if it
> > was
> > + * set to 0.
> > + */
> > + int apply_cropping;
>
> Why is this a new field? As long as it is used to convey a boolean
> state isn't a codec flag more efficient?
I'm not a big fan of the flags field, as it mushes together a bunch of
completely unrelated functionality, a lot of it encoding options.
>
> > } AVCodecContext;
> >
> > /**
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index 0fd41ab..65ee8b0 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -29,6 +29,7 @@
> > #include "libavutil/frame.h"
> > #include "libavutil/hwcontext.h"
> > #include "libavutil/imgutils.h"
> > +#include "libavutil/intmath.h"
> >
> > #include "avcodec.h"
> > #include "bytestream.h"
> > @@ -450,6 +451,106 @@ int attribute_align_arg
> > avcodec_send_packet(AVCodecContext *avctx, const AVPacke
> > return 0;
> > }
> >
> > +static int calc_cropping_offsets(size_t offsets[4], const AVFrame *frame,
> > + const AVPixFmtDescriptor *desc)
>
> IMO passing desc here is not ideal, deriving it from frame seems more
> usual to me
How is it better? What's the advantage?
>
> > +{
> > + int i, j;
> > +
> > + for (i = 0; frame->data[i]; i++) {
> > + const AVComponentDescriptor *comp = NULL;
> > + int shift_x = (i == 1 || i == 2) ? desc->log2_chroma_w : 0;
> > + int shift_y = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
> > +
> > + if (desc->flags & (AV_PIX_FMT_FLAG_PAL |
> > AV_PIX_FMT_FLAG_PSEUDOPAL) && i == 1) {
> > + offsets[i] = 0;
> > + break;
> > + }
> > +
> > + /* find any component descriptor for this plane */
> > + for (j = 0; j < desc->nb_components; j++) {
> > + if (desc->comp[j].plane == i) {
> > + comp = &desc->comp[j];
> > + break;
> > + }
> > + }
> > + if (!comp)
> > + return AVERROR_BUG;
> > +
> > + offsets[i] = (frame->crop_top >> shift_y) * frame->linesize[i] +
> > + (frame->crop_left >> shift_x) * comp->step;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int apply_cropping(AVCodecContext *avctx, AVFrame *frame)
> > +{
> > + const AVPixFmtDescriptor *desc;
> > + size_t offsets[4];
> > + int i;
> > +
> > + /* make sure we are noisy about decoders returning invalid cropping
> > data */
> > + if (frame->crop_left >= INT_MAX - frame->crop_right ||
> > + frame->crop_top >= INT_MAX - frame->crop_bottom ||
> > + (frame->crop_left + frame->crop_right) >= frame->width ||
> > + (frame->crop_top + frame->crop_bottom) >= frame->height) {
> > + av_log(avctx, AV_LOG_WARNING,
> > + "Invalid cropping information set by a decoder:
> > %zu/%zu/%zu/%zu "
> > + "(frame size %dx%d). This is a bug, please report it\n",
> > + frame->crop_left, frame->crop_right, frame->crop_top,
> > frame->crop_bottom,
> > + frame->width, frame->height);
> > + frame->crop_left = 0;
> > + frame->crop_right = 0;
> > + frame->crop_top = 0;
> > + frame->crop_bottom = 0;
> > + return 0;
> > + }
> > +
> > + if (!avctx->apply_cropping)
> > + return 0;
> > +
> > + desc = av_pix_fmt_desc_get(frame->format);
> > + if (!desc)
> > + return AVERROR_BUG;
> > +
> > + /* Do nothing for hwaccel formats.
> > + * Bitstream formats cannot be easily handled here either (and
> > corresponding
> > + * decoders should not export any cropping anyway), so also do nothing
> > for
> > + * those. */
> > + if (desc->flags & (AV_PIX_FMT_FLAG_BITSTREAM |
> > AV_PIX_FMT_FLAG_HWACCEL))
> > + return 0;
> > +
> > + /* calculate the offsets for each plane */
> > + calc_cropping_offsets(offsets, frame, desc);
>
> unchecked return value
right
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
