[FFmpeg-devel] [crop support for matroska demuxer 2/3] libavcodec: Public code to support container crop

[Hendrik Leppkes]

On Sat, Oct 1, 2022 at 8:14 AM OvchinnikovDmitrii
<ovchinnikov.dmitrii at gmail.com> wrote:
>
> Support both simple and receive_frame api
> The container crop information is applied additional to frame crop information
> ---
>  libavcodec/codec_par.c |  8 ++++++++
>  libavcodec/decode.c    | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
> index abda649aa8..f74964a817 100644
> --- a/libavcodec/codec_par.c
> +++ b/libavcodec/codec_par.c
> @@ -118,6 +118,10 @@ int avcodec_parameters_from_context(AVCodecParameters *par,
>          par->format              = codec->pix_fmt;
>          par->width               = codec->width;
>          par->height              = codec->height;
> +        par->crop_top           = codec->crop_top;
> +        par->crop_left          = codec->crop_left;
> +        par->crop_bottom        = codec->crop_bottom;
> +        par->crop_right         = codec->crop_right;
>          par->field_order         = codec->field_order;
>          par->color_range         = codec->color_range;
>          par->color_primaries     = codec->color_primaries;
> @@ -199,6 +203,10 @@ int avcodec_parameters_to_context(AVCodecContext *codec,
>          codec->pix_fmt                = par->format;
>          codec->width                  = par->width;
>          codec->height                 = par->height;
> +        codec->crop_top               = par->crop_top;
> +        codec->crop_left              = par->crop_left;
> +        codec->crop_bottom            = par->crop_bottom;
> +        codec->crop_right             = par->crop_right;
>          codec->field_order            = par->field_order;
>          codec->color_range            = par->color_range;
>          codec->color_primaries        = par->color_primaries;
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 6be2d3d6ed..548225c904 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -324,6 +324,16 @@ static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame,
>      emms_c();
>      actual_got_frame = got_frame;
>
> +    /* crop for simple api mode. apply additional container crop info to frame */
> +    if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> +        if (avctx->crop_top != 0 || avctx->crop_left != 0 || avctx->crop_right != 0 || avctx->crop_bottom != 0){
> +            frame->crop_top    += avctx->crop_top;
> +            frame->crop_left   += avctx->crop_left;
> +            frame->crop_right  += avctx->crop_right;
> +            frame->crop_bottom += avctx->crop_bottom;
> +        }
> +    }
> +
>      if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>          if (frame->flags & AV_FRAME_FLAG_DISCARD)
>              got_frame = 0;
> @@ -707,6 +717,16 @@ int ff_decode_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>
>      if (avci->buffer_frame->buf[0]) {
>          av_frame_move_ref(frame, avci->buffer_frame);
> +
> +        /* crop for receive_frame api mode. apply additional container crop info to frame */
> +        if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> +            if (avctx->crop_top != 0 || avctx->crop_left != 0 || avctx->crop_right != 0 || avctx->crop_bottom != 0){
> +                frame->crop_top    += avctx->crop_top;
> +                frame->crop_left   += avctx->crop_left;
> +                frame->crop_right  += avctx->crop_right;
> +                frame->crop_bottom += avctx->crop_bottom;
> +            }
> +        }

Somehow I don't feel like adding the two crops together is really
going to do what most users would expect to happen. It just feels
weird.
It also changes the decoder crop information, and an API user does not
get the pure information from the decoder, but rather an "interpreted"
form. As an API user, I do not get the ability here to extract the
pure decoder crop info, unless I manually null out the container info
beforehand, or subtract it again, both of which seems odd and
undesirable to me. Shouldn't I be provided with clean information, and
then I (the API user) can choose how to act?

- Hendrik