Quoting wm4 (2016-12-30 10:41:58)
> On Tue, 27 Dec 2016 19:31:15 +0100
> Anton Khirnov <[email protected]> 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.
>
> Maybe could explain why AVCodecContext or AVCodecPar width/height will
> mismatch with the width/height of the AVFrame if it's set to 0. From
> what I understand, containers will normally use a cropped dimension (?).
AFAIK most containers just store "dimensions" without further
explanation, usually interpreted as visible (since coded is not really
meaningful outside of the decoding process).
>
> > + *
> > + * 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.
> > + */
>
> Isn't that last part an implicit API change? This would probably annoy
> a lot of people.
Right, didn't think this through. Will fix.
>
> > + int apply_cropping;
> > } 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)
> > +{
> > + 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 ||
>
> These fields are size_t, so why INT_MAX? Sure, the frame size can't be
> larger than INT_MAX anyway, but this is a good opportunity for
> nitpicking.
frame->width/height is still int.
>
> > + (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);
> > +
> > + /* adjust the offsets to avoid breaking alignment */
> > + if (!(avctx->flags & AV_CODEC_FLAG_UNALIGNED)) {
> > + int min_log2_align = INT_MAX;
> > +
> > + for (i = 0; frame->data[i]; i++) {
> > + int log2_align = offsets[i] ? av_ctz(offsets[i]) : INT_MAX;
> > + min_log2_align = FFMIN(log2_align, min_log2_align);
> > + }
> > +
> > + if (min_log2_align < 5) {
> > + frame->crop_left &= ~((1 << min_log2_align) - 1);
> > + calc_cropping_offsets(offsets, frame, desc);
> > + }
> > + }
> > +
> > + for (i = 0; frame->data[i]; i++)
> > + frame->data[i] += offsets[i];
> > +
> > + frame->width -= (frame->crop_left + frame->crop_right);
> > + frame->height -= (frame->crop_top + frame->crop_bottom);
> > + frame->crop_left = 0;
> > + frame->crop_right = 0;
> > + frame->crop_top = 0;
> > + frame->crop_bottom = 0;
> > +
> > + return 0;
> > +}
> > +
> > int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx,
> > AVFrame *frame)
> > {
> > AVCodecInternal *avci = avctx->internal;
> > @@ -472,6 +573,14 @@ int attribute_align_arg
> > avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
> > return ret;
> > }
> >
> > + if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
> > + ret = apply_cropping(avctx, frame);
> > + if (ret < 0) {
> > + av_frame_unref(frame);
> > + return ret;
> > + }
> > + }
> > +
> > avctx->frame_number++;
> >
> > return 0;
> > @@ -1029,7 +1138,8 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame
> > *frame, int flags)
> > ret = avctx->get_buffer2(avctx, frame, flags);
> >
> > end:
> > - if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions) {
> > + if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions &&
> > + !(avctx->codec->caps_internal & FF_CODEC_CAP_EXPORTS_CROPPING)) {
> > frame->width = avctx->width;
> > frame->height = avctx->height;
>
> Hm, not sure what's happening here. The existing code looks a bit
> fishy. But I guess excluding this if cropping is "properly" done is the
> right thing.
override_dimensions=1 means that the decoder set the frame dimensions
manually, so we don't touch them at all here (not sure if any decoders
still do this).
When override_dimensions=0, we set the frame dimensions to coded one
above, allocate the buffer and then replace them with visible ones here.
If the decoder exports cropping then the final replacement shouldn't
happen, since the frame width/height now store coded values.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
