On Tue, 27 Dec 2016 19:31:15 +0100
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.
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 (?).
> + *
> + * 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.
> + 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->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.
> }
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 796d45f..5b82504 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -53,6 +53,12 @@
> * from the input AVPacket.
> */
> #define FF_CODEC_CAP_SETS_PKT_DTS (1 << 2)
> +/**
> + * The decoder sets the cropping fields in the output frames manually.
> + * If this cap is set, the generic code will initialize output frame
> + * dimensions to coded rather than display values.
> + */
> +#define FF_CODEC_CAP_EXPORTS_CROPPING (1 << 3)
>
> #ifdef DEBUG
> # define ff_dlog(ctx, ...) av_log(ctx, AV_LOG_DEBUG, __VA_ARGS__)
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 4deb223..3ac53fb 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -531,6 +531,7 @@ static const AVOption avcodec_options[] = {
> #if FF_API_SIDEDATA_ONLY_PKT
> {"side_data_only_packets", NULL, OFFSET(side_data_only_packets),
> AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, A|V|E },
> #endif
> +{"apply_cropping", NULL, OFFSET(apply_cropping), AV_OPT_TYPE_INT, { .i64 = 1
> }, 0, 1, V | D },
> {NULL},
> };
>
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 5b6fb6c..df0c01f 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,8 +28,8 @@
> #include "libavutil/version.h"
>
> #define LIBAVCODEC_VERSION_MAJOR 57
> -#define LIBAVCODEC_VERSION_MINOR 30
> -#define LIBAVCODEC_VERSION_MICRO 2
> +#define LIBAVCODEC_VERSION_MINOR 31
> +#define LIBAVCODEC_VERSION_MICRO 0
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> LIBAVCODEC_VERSION_MINOR, \
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
