Re: [FFmpeg-devel] [PATCH] avcodec/amf_enc: av1 cropping support

[James Almer <jamrial@gmail.com>]

On 7/23/2024 6:53 PM, Araz Iusubov wrote:
> ---
>   libavcodec/amfenc_av1.c | 82 ++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 81 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/amfenc_av1.c b/libavcodec/amfenc_av1.c
> index d40c71cb33..27599b9fbe 100644
> --- a/libavcodec/amfenc_av1.c
> +++ b/libavcodec/amfenc_av1.c
> @@ -22,6 +22,9 @@
>   #include "amfenc.h"
>   #include "codec_internal.h"
>   
> +#define AMF_VIDEO_ENCODER_AV1_CAP_WIDTH_ALIGNMENT_FACTOR_LOCAL                L"Av1WidthAlignmentFactor"          // amf_int64; default = 1
> +#define AMF_VIDEO_ENCODER_AV1_CAP_HEIGHT_ALIGNMENT_FACTOR_LOCAL               L"Av1HeightAlignmentFactor"         // amf_int64; default = 1
> +
>   #define OFFSET(x) offsetof(AmfContext, x)
>   #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>   static const AVOption options[] = {
> @@ -167,7 +170,13 @@ static av_cold int amf_encode_init_av1(AVCodecContext* avctx)
>       AMFRate             framerate;
>       AMFSize             framesize = AMFConstructSize(avctx->width, avctx->height);
>   
> -
> +    //for av1 alignment and crop
> +    AVPacketSideData*   sd_crop = NULL;
> +    uint32_t*           crop    = NULL;
> +    uint32_t            crop_right  = 0;
> +    uint32_t            crop_bottom = 0;
> +    int                 width_alignment_factor  = -1;
> +    int                 height_alignment_factor = -1;
>   
>       if (avctx->framerate.num > 0 && avctx->framerate.den > 0) {
>           framerate = AMFConstructRate(avctx->framerate.num, avctx->framerate.den);
> @@ -482,6 +491,77 @@ FF_ENABLE_DEPRECATION_WARNINGS
>       buffer->pVtbl->Release(buffer);
>       var.pInterface->pVtbl->Release(var.pInterface);
>   
> +    //processing crop informaiton according to alignment
> +    if (ctx->encoder->pVtbl->GetProperty(ctx->encoder, AMF_VIDEO_ENCODER_AV1_CAP_WIDTH_ALIGNMENT_FACTOR_LOCAL, &var) != AMF_OK)
> +    {

Don't put the brackets in their own line.

> +        // assume older driver and Navi3x
> +        width_alignment_factor = 64;
> +    }
> +    else
> +    {

And don't use brackets for a single line block.

> +        width_alignment_factor = (int)var.int64Value;
> +    }
> +
> +    if (ctx->encoder->pVtbl->GetProperty(ctx->encoder, AMF_VIDEO_ENCODER_AV1_CAP_HEIGHT_ALIGNMENT_FACTOR_LOCAL, &var) != AMF_OK)
> +    {
> +        // assume older driver and Navi3x
> +        height_alignment_factor = 16;
> +    }
> +    else
> +    {
> +        height_alignment_factor = (int)var.int64Value;
> +    }
> +
> +    if (width_alignment_factor != -1 &&  height_alignment_factor != -1)
> +    {
> +        if (avctx->width % width_alignment_factor != 0)
> +        {
> +            crop_right = width_alignment_factor - (avctx->width & (width_alignment_factor - 1));
> +        }
> +
> +        if (avctx->height % height_alignment_factor != 0)
> +        {
> +            crop_bottom = height_alignment_factor - (avctx->height & (height_alignment_factor - 1));
> +        }
> +
> +        //There is specia processing for crop_bottom equal to 8 in hardware
> +        if (crop_bottom == 8)
> +        {
> +            crop_bottom = 2;
> +        }
> +    }
> +
> +    if (crop_right != 0 || crop_bottom != 0)
> +    {
> +        sd_crop = av_realloc_array(avctx->coded_side_data, avctx->nb_coded_side_data + 1, sizeof(*sd_crop));
> +        if (!sd_crop)
> +        {
> +            av_log(ctx, AV_LOG_WARNING, "Can't allocate memory for amf av1 encoder crop information\n");
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        sd_crop->data = (uint8_t*)av_mallocz(sizeof(uint32_t) * 4);

This is setting data from the very first side data entry, not the last. 
You're then setting type and size in the last at the end of this block.

> +        if (!sd_crop->data)
> +        {
> +            av_log(ctx, AV_LOG_WARNING, "Can't allocate memory for amf av1 encoder crop information\n");
> +            return AVERROR(ENOMEM);

If you return here, avctx->coded_side_data will have a stale pointer.

> +        }
> +
> +        crop = (uint32_t*)sd_crop->data;
> +
> +        //top, bottom, left,right
> +        *crop++ = 0;
> +        *crop++ = crop_bottom;
> +        *crop++ = 0;
> +        *crop   = crop_right;

Just to be safe, use AV_WL32() for these, even if no big endian system 
supports this encoder.

> +
> +        avctx->nb_coded_side_data++;
> +
> +        avctx->coded_side_data = sd_crop;
> +        avctx->coded_side_data[avctx->nb_coded_side_data - 1].type = AV_PKT_DATA_FRAME_CROPPING;
> +        avctx->coded_side_data[avctx->nb_coded_side_data - 1].size = sizeof(uint32_t) * 4;
> +    }
> +
>       return 0;
>   }
>   
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".