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

---
 libavcodec/amfenc.c     |  6 ++++
 libavcodec/amfenc.h     |  3 ++
 libavcodec/amfenc_av1.c | 64 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 72 insertions(+), 1 deletion(-)

> On Jul 25, 2024, at 20:06, Araz Iusubov <primeadvice@gmail.com> wrote:
> 
> ---
> libavcodec/amfenc.c     |  6 ++++
> libavcodec/amfenc.h     |  3 ++
> libavcodec/amfenc_av1.c | 64 ++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 061859f85c..93925854e0 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -402,6 +402,12 @@ int av_cold ff_amf_encode_close(AVCodecContext *avctx)
>         dlclose(ctx->library);
>         ctx->library = NULL;
>     }
> +
> +    if (ctx->crop) 
> +    {
> +        av_freep(ctx->crop);
> +    }
> +

Here should be av_freep(&ctx->crop), without ‘if’.

But why use a pointer than uint32_t crop[4]?

>     ctx->trace = NULL;
>     ctx->debug = NULL;
>     ctx->factory = NULL;
> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> index 2dbd378ef8..abcf465063 100644
> --- a/libavcodec/amfenc.h
> +++ b/libavcodec/amfenc.h
> @@ -81,6 +81,9 @@ typedef struct AmfContext {
> 
>     int                 log_to_dbg;
> 
> +    //handle crop
> +    uint32_t            *crop;
> +
>     // Static options, have to be set before Init() call
>     int                 usage;
>     int                 profile;
> diff --git a/libavcodec/amfenc_av1.c b/libavcodec/amfenc_av1.c
> index d40c71cb33..e9bc26a770 100644
> --- a/libavcodec/amfenc_av1.c
> +++ b/libavcodec/amfenc_av1.c
> @@ -19,9 +19,13 @@
> #include "libavutil/internal.h"
> #include "libavutil/mem.h"
> #include "libavutil/opt.h"
> +#include "libavutil/intreadwrite.h"
> #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 +171,12 @@ 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_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,59 @@ 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)
> +        // assume older driver and Navi3x
> +        width_alignment_factor = 64;
> +    else
> +        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) {
> +        ctx->crop = av_mallocz(sizeof(uint32_t) * 4);
> +        if (!ctx->crop) {
> +            av_log(ctx, AV_LOG_WARNING, "Can't allocate memory for amf av1 encoder crop information\n");
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        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");
> +            av_freep(ctx->crop);
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        avctx->coded_side_data = sd_crop;
> +        avctx->nb_coded_side_data++;
> +
> +        //top, bottom, left,right
> +        AV_WL32(ctx->crop + 0, 0);
> +        AV_WL32(ctx->crop + 1, crop_bottom);
> +        AV_WL32(ctx->crop + 2, 0);
> +        AV_WL32(ctx->crop + 3, crop_right);
> +
> +        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].data = (uint8_t*)ctx->crop;
> +        avctx->coded_side_data[avctx->nb_coded_side_data - 1].size = sizeof(uint32_t) * 4;
> +    }
> +
>     return 0;
> }
> 
> -- 
> 2.45.2.windows.1
> 
> _______________________________________________
> 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".

On 7/25/2024 9:06 AM, Araz Iusubov wrote:
> ---
>   libavcodec/amfenc.c     |  6 ++++
>   libavcodec/amfenc.h     |  3 ++
>   libavcodec/amfenc_av1.c | 64 ++++++++++++++++++++++++++++++++++++++++-
>   3 files changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 061859f85c..93925854e0 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -402,6 +402,12 @@ int av_cold ff_amf_encode_close(AVCodecContext *avctx)
>           dlclose(ctx->library);
>           ctx->library = NULL;
>       }
> +
> +    if (ctx->crop)
> +    {
> +        av_freep(ctx->crop);
> +    }
> +
>       ctx->trace = NULL;
>       ctx->debug = NULL;
>       ctx->factory = NULL;
> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> index 2dbd378ef8..abcf465063 100644
> --- a/libavcodec/amfenc.h
> +++ b/libavcodec/amfenc.h
> @@ -81,6 +81,9 @@ typedef struct AmfContext {
>   
>       int                 log_to_dbg;
>   
> +    //handle crop
> +    uint32_t            *crop;
> +
>       // Static options, have to be set before Init() call
>       int                 usage;
>       int                 profile;
> diff --git a/libavcodec/amfenc_av1.c b/libavcodec/amfenc_av1.c
> index d40c71cb33..e9bc26a770 100644
> --- a/libavcodec/amfenc_av1.c
> +++ b/libavcodec/amfenc_av1.c
> @@ -19,9 +19,13 @@
>   #include "libavutil/internal.h"
>   #include "libavutil/mem.h"
>   #include "libavutil/opt.h"
> +#include "libavutil/intreadwrite.h"
>   #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 +171,12 @@ 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_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,59 @@ 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)
> +        // assume older driver and Navi3x
> +        width_alignment_factor = 64;
> +    else
> +        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) {
> +        ctx->crop = av_mallocz(sizeof(uint32_t) * 4);
> +        if (!ctx->crop) {
> +            av_log(ctx, AV_LOG_WARNING, "Can't allocate memory for amf av1 encoder crop information\n");
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        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");
> +            av_freep(ctx->crop);
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        avctx->coded_side_data = sd_crop;
> +        avctx->nb_coded_side_data++;
> +
> +        //top, bottom, left,right
> +        AV_WL32(ctx->crop + 0, 0);
> +        AV_WL32(ctx->crop + 1, crop_bottom);
> +        AV_WL32(ctx->crop + 2, 0);
> +        AV_WL32(ctx->crop + 3, crop_right);
> +
> +        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].data = (uint8_t*)ctx->crop;

You're passing ownership of this buffer to the packet side data array 
yet still try to free it in ff_amf_encode_close() because you kept the 
pointer around in the encoder's context.

In any case, I've fixed this and pushed the patch.
Thanks.

> +        avctx->coded_side_data[avctx->nb_coded_side_data - 1].size = sizeof(uint32_t) * 4;
> +    }
> +
>       return 0;
>   }
>