[FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process

James Almer jamrial at gmail.com
Thu Apr 5 19:14:00 EEST 2018


On 4/5/2018 12:23 PM, Alexander Kravchenko wrote:
> 
> This fixes frame corruption issue when decoder started reusing frames while they are still in use of encoding process
> Issue with frame corruption  was reproduced using:
> ffmpeg.exe -y -hwaccel d3d11va -hwaccel_output_format d3d11 -i input.h264  -an -c:v h264_amf output.mkv
> 
> Previous questions and answers
>> How many frames can end up queued inside the encoder here?
> 16
> 
>> Is there always an exact 1->1 correspondence between input frames and output packets?  That is, is it guaranteed that no frames
> are
>> ever dropped, even in the low-latency modes?
> yes
> 
>> These look even more like they should be functions rather than macros now?  In particular, the returns in them interact badly with
> the code below.
> Macroses were used because they works with different types. I have removed returns from them (sorry, I missed this comment).
> 
> ---
>  libavcodec/amfenc.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
> 
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 89a10ff253..084b06e56d 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -443,6 +443,73 @@ int ff_amf_encode_init(AVCodecContext *avctx)
>      return ret;
>  }
>  
> +#define AMF_AV_QUERY_INTERFACE(res, from, InterfaceTypeTo, to) \
> +    { \
> +        AMFGuid guid_##InterfaceTypeTo = IID_##InterfaceTypeTo(); \
> +        res = from->pVtbl->QueryInterface(from, &guid_##InterfaceTypeTo, (void**)&to); \
> +    }
> +
> +#define AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, pThis, name, val) \
> +    { \
> +        AMFInterface *amf_interface; \
> +        AMFVariantStruct var; \
> +        res = AMFVariantInit(&var); \
> +        if (res == AMF_OK) { \
> +            AMF_AV_QUERY_INTERFACE(res, val, AMFInterface, amf_interface)\
> +            if (res == AMF_OK) { \
> +                res = AMFVariantAssignInterface(&var, amf_interface); \
> +                amf_interface->pVtbl->Release(amf_interface); \
> +            } \
> +            if (res == AMF_OK) { \
> +                res = pThis->pVtbl->SetProperty(pThis, name, var); \
> +            } \
> +            AMFVariantClear(&var); \
> +        }\
> +    }
> +
> +#define AMF_AV_GET_PROPERTY_INTERFACE(res, pThis, name, TargetType, val) \
> +    { \
> +        AMFVariantStruct var; \
> +        res = AMFVariantInit(&var); \
> +        if (res == AMF_OK) { \
> +            res = pThis->pVtbl->GetProperty(pThis, name, &var); \
> +            if (res == AMF_OK) { \
> +                if (var.type == AMF_VARIANT_INTERFACE && AMFVariantInterface(&var)) { \
> +                    AMF_AV_QUERY_INTERFACE(res, AMFVariantInterface(&var), TargetType, val); \
> +                } else { \
> +                    res = AMF_INVALID_DATA_TYPE; \
> +                } \
> +            } \
> +            AMFVariantClear(&var); \
> +        }\
> +    }
> +
> +static AMFBuffer* amf_create_buffer_with_frame_ref(const AVFrame* frame, AMFContext *context)
> +{
> +    AVFrame *frame_ref;
> +    AMFBuffer *frame_ref_storage_buffer = NULL;
> +    AMF_RESULT res;
> +
> +    res = context->pVtbl->AllocBuffer(context, AMF_MEMORY_HOST, sizeof(frame_ref), &frame_ref_storage_buffer);
> +    if (res == AMF_OK) {
> +        frame_ref = av_frame_clone(frame);
> +        if (frame_ref) {
> +            memcpy(frame_ref_storage_buffer->pVtbl->GetNative(frame_ref_storage_buffer), &frame_ref, sizeof(frame_ref));
> +        } else {
> +            frame_ref_storage_buffer->pVtbl->Release(frame_ref_storage_buffer);
> +            frame_ref_storage_buffer = NULL;
> +        }
> +    }
> +    return frame_ref_storage_buffer;
> +}
> +
> +static void amf_release_buffer_with_frame_ref(AMFBuffer *frame_ref_storage_buffer)
> +{
> +    AVFrame *av_frame_ref;
> +    memcpy(&av_frame_ref, frame_ref_storage_buffer->pVtbl->GetNative(frame_ref_storage_buffer), sizeof(av_frame_ref));
> +    av_frame_free(&av_frame_ref);
> +    frame_ref_storage_buffer->pVtbl->Release(frame_ref_storage_buffer);
> +} 
>  
>  int ff_amf_send_frame(AVCodecContext *avctx, const AVFrame *frame)
>  {
> @@ -484,6 +551,7 @@ int ff_amf_send_frame(AVCodecContext *avctx, const AVFrame *frame)
>              (ctx->hw_device_ctx && ((AVHWFramesContext*)frame->hw_frames_ctx->data)->device_ctx ==
>              (AVHWDeviceContext*)ctx->hw_device_ctx->data)
>          )) {
> +            AMFBuffer *frame_ref_storage_buffer;
>  #if CONFIG_D3D11VA
>              static const GUID AMFTextureArrayIndexGUID = { 0x28115527, 0xe7c3, 0x4b66, { 0x99, 0xd3, 0x4f, 0x2a, 0xe6, 0xb4, 0x7f,
> 0xaf } };
>              ID3D11Texture2D *texture = (ID3D11Texture2D*)frame->data[0]; // actual texture
> @@ -496,6 +564,16 @@ int ff_amf_send_frame(AVCodecContext *avctx, const AVFrame *frame)
>              // input HW surfaces can be vertically aligned by 16; tell AMF the real size
>              surface->pVtbl->SetCrop(surface, 0, 0, frame->width, frame->height);
>  #endif
> +
> +            frame_ref_storage_buffer = amf_create_buffer_with_frame_ref(frame, ctx->context);
> +            AMF_RETURN_IF_FALSE(ctx, frame_ref_storage_buffer != NULL, AVERROR(ENOMEM), "create_buffer_with_frame_ref() returned
> NULL\n");
> +
> +            AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, surface, L"av_frame_ref", frame_ref_storage_buffer);
> +            if (res != AMF_OK) {
> +                surface->pVtbl->Release(surface);
> +            }
> +            frame_ref_storage_buffer->pVtbl->Release(frame_ref_storage_buffer);
> +            AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR(ENOMEM), "SetProperty failed for \"av_frame_ref\" with error %d\n",
> res);
>          } else {
>              res = ctx->context->pVtbl->AllocSurface(ctx->context, AMF_MEMORY_HOST, ctx->format, avctx->width, avctx->height,
> &surface);
>              AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR(ENOMEM), "AllocSurface() failed  with error %d\n", res);
> @@ -560,6 +638,17 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt)
>              ret = amf_copy_buffer(avctx, avpkt, buffer);
>  
>              buffer->pVtbl->Release(buffer);
> +
> +            if (data->pVtbl->HasProperty(data, L"av_frame_ref")) {
> +                AMFBuffer *frame_ref_storage_buffer;
> +                AMF_AV_GET_PROPERTY_INTERFACE(res, data, L"av_frame_ref", AMFBuffer, frame_ref_storage_buffer);
> +                if (res == AMF_OK) {
> +                    amf_release_buffer_with_frame_ref(frame_ref_storage_buffer);
> +                } else {
> +                    av_log(avctx, AV_LOG_WARNING, "GetProperty failed for \"av_frame_ref\" with error %d\n", res);
> +                }
> +            }
> +
>              data->pVtbl->Release(data);
>  
>              AMF_RETURN_IF_FALSE(ctx, ret >= 0, ret, "amf_copy_buffer() failed with error %d\n", ret);
> 

This breaks the testcase described in
https://trac.ffmpeg.org/ticket/6990 which is basically the same as the
one you described in this patch.

I get the following spammed repeatedly:

[AVHWFramesContext @ 000000000502d340] Static surface pool size exceeded.
[mpeg2video @ 0000000002f8d400] get_buffer() failed
[mpeg2video @ 0000000002f8d400] thread_get_buffer() failed
[mpeg2video @ 0000000002f8d400] get_buffer() failed (-12 0000000000000000)
Error while decoding stream #0:1: Operation not permitted


More information about the ffmpeg-devel mailing list