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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
