Message ID | 004701d3d022$99422230$cbc66690$@gmail.com |
---|---|
State | New |
Headers | show |
On 09/04/18 17:48, Alexander Kravchenko wrote: > Hi, could you please review updated patch? > > Fixes according on Mark's review: > * Macroses changed to functions > * error level of AMF_RETURN_IF_FALSE changed to fatal (all cases it returns are fatal according on fatal error level description) > * used AMF_RETURN_IF_FALSE for case if a frame reference has been completely lost (was just warning before) > > Hopefully this patch is ok enough to be applied :). Two minor points below, not enough to merit another cycle so I fixed them and pushed. > FYI, near time I am going to send the following patches > * cosmetic fixes > * hwcontext_amf (to be reused in encoder and color space converter) > * AMF colors space converter (input memory types: opencl, dx9, dx11, host; output memory types: opencl, dx9, dx11) > > > --- ^ (When adding commentary which isn't part of the commit message to an email please place it after this line so that it doesn't end up in the commit message.) > libavcodec/amfenc.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++- > libavcodec/amfenc.h | 5 ++- > 2 files changed, 97 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c > index 89a10ff253..ea2c5acbbf 100644 > --- a/libavcodec/amfenc.c > +++ b/libavcodec/amfenc.c > @@ -157,6 +157,9 @@ static int amf_init_context(AVCodecContext *avctx) > AmfContext *ctx = avctx->priv_data; > AMF_RESULT res = AMF_OK; > > + ctx->hwsurfaces_in_queue = 0; > + ctx->hwsurfaces_in_queue_max = 16; > + > // configure AMF logger > // the return of these functions indicates old state and do not affect behaviour > ctx->trace->pVtbl->EnableWriter(ctx->trace, AMF_TRACE_WRITER_DEBUG_OUTPUT, ctx->log_to_dbg != 0 ); > @@ -187,6 +190,7 @@ static int amf_init_context(AVCodecContext *avctx) > if (!ctx->hw_frames_ctx) { > return AVERROR(ENOMEM); > } > + ctx->hwsurfaces_in_queue_max = device_ctx->initial_pool_size - 1; initial_pool_size need not be set to a positive value. It will be set for all internally-created frames contexts (since the setup code there is intended for use with the decoder), but it need not be for externally-created one which might not use array textures and therefore need not have fixed size. Changed to only update the queue size if initial_pool_size is positive (i.e. use the 16 length if the pool isn't of fixed size). > } else { > if(res == AMF_NOT_SUPPORTED) > av_log(avctx, AV_LOG_INFO, "avctx->hw_frames_ctx has D3D11 device which doesn't have D3D11VA interface, switching to default\n"); > @@ -443,6 +447,75 @@ int ff_amf_encode_init(AVCodecContext *avctx) > return ret; > } > > +static AMF_RESULT amf_set_property_buffer(AMFSurface *object, const wchar_t *name, AMFBuffer *val) > +{ > + AMF_RESULT res; > + AMFVariantStruct var; > + res = AMFVariantInit(&var); > + if (res == AMF_OK) { > + AMFGuid guid_AMFInterface = IID_AMFInterface(); > + AMFInterface *amf_interface; > + res = val->pVtbl->QueryInterface(val, &guid_AMFInterface, (void**)&amf_interface); > + > + if (res == AMF_OK) { > + res = AMFVariantAssignInterface(&var, amf_interface); > + amf_interface->pVtbl->Release(amf_interface); > + } > + if (res == AMF_OK) { > + res = object->pVtbl->SetProperty(object, name, var); > + } > + AMFVariantClear(&var); > + } > + return res; > +} > + > +static AMF_RESULT amf_get_property_buffer(AMFData *object, const wchar_t *name, AMFBuffer **val) > +{ > + AMF_RESULT res; > + AMFVariantStruct var; > + res = AMFVariantInit(&var); > + if (res == AMF_OK) { > + res = object->pVtbl->GetProperty(object, name, &var); > + if (res == AMF_OK) { > + if (var.type == AMF_VARIANT_INTERFACE) { > + AMFGuid guid_AMFBuffer = IID_AMFBuffer(); > + AMFInterface *amf_interface = AMFVariantInterface(&var); > + res = amf_interface->pVtbl->QueryInterface(amf_interface, &guid_AMFBuffer, (void**)val); > + } else { > + res = AMF_INVALID_DATA_TYPE; > + } > + } > + AMFVariantClear(&var); > + } > + return res; > +} > + > +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 +557,8 @@ 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 +571,14 @@ 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"); > + > + res = amf_set_property_buffer(surface, L"av_frame_ref", frame_ref_storage_buffer); > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "SetProperty failed for \"av_frame_ref\" with error %d\n", res); > + ctx->hwsurfaces_in_queue++; > + frame_ref_storage_buffer->pVtbl->Release(frame_ref_storage_buffer); > } 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 +643,15 @@ 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; > + res = amf_get_property_buffer(data, L"av_frame_ref", &frame_ref_storage_buffer); > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetProperty failed for \"av_frame_ref\" with error %d\n", res); > + amf_release_buffer_with_frame_ref(frame_ref_storage_buffer); > + ctx->hwsurfaces_in_queue--; > + } > + > data->pVtbl->Release(data); > > AMF_RETURN_IF_FALSE(ctx, ret >= 0, ret, "amf_copy_buffer() failed with error %d\n", ret); > @@ -589,7 +681,7 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt) > av_log(avctx, AV_LOG_WARNING, "Data acquired but delayed drain submission got AMF_INPUT_FULL- should not happen\n"); > } > } > - } else if (ctx->delayed_surface != NULL || ctx->delayed_drain || (ctx->eof && res_query != AMF_EOF)) { > + } else if (ctx->delayed_surface != NULL || ctx->delayed_drain || (ctx->eof && res_query != AMF_EOF) || (ctx->hwsurfaces_in_queue >= ctx->hwsurfaces_in_queue_max)) { > block_and_wait = 1; > av_usleep(1000); // wait and poll again > } > diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h > index 84f0aad2fa..0e4c6e4b89 100644 > --- a/libavcodec/amfenc.h > +++ b/libavcodec/amfenc.h > @@ -62,6 +62,9 @@ typedef struct AmfContext { > AVBufferRef *hw_device_ctx; ///< pointer to HW accelerator (decoder) > AVBufferRef *hw_frames_ctx; ///< pointer to HW accelerator (frame allocator) > > + int hwsurfaces_in_queue; > + int hwsurfaces_in_queue_max; > + > // helpers to handle async calls > int delayed_drain; > AMFSurface *delayed_surface; > @@ -140,7 +143,7 @@ extern const enum AVPixelFormat ff_amf_pix_fmts[]; > */ > #define AMF_RETURN_IF_FALSE(avctx, exp, ret_value, /*message,*/ ...) \ > if (!(exp)) { \ > - av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \ > + av_log(avctx, AV_LOG_FATAL, __VA_ARGS__); \ This isn't related, I removed it. > return ret_value; \ > } > > Thanks, - Mark
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson > Sent: Wednesday, April 11, 2018 2:13 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process > > > Two minor points below, not enough to merit another cycle so I fixed them and pushed. > Hi Mark, Thanks for your help reviewing, fixing, and pushing patch I have send one more patch with subject: [PATCH] lavc/amfenc: DXVA2 textures support implementation by AMF encoder http://ffmpeg.org/pipermail/ffmpeg-devel/2018-April/228186.html Could you please review it? FYI, my next steps are: * hwcontext_amf + apply using one in amfenc. * vf_scale_amf * code cleanup Thanks, Alexander
diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c index 89a10ff253..ea2c5acbbf 100644 --- a/libavcodec/amfenc.c +++ b/libavcodec/amfenc.c @@ -157,6 +157,9 @@ static int amf_init_context(AVCodecContext *avctx) AmfContext *ctx = avctx->priv_data; AMF_RESULT res = AMF_OK; + ctx->hwsurfaces_in_queue = 0; + ctx->hwsurfaces_in_queue_max = 16; + // configure AMF logger // the return of these functions indicates old state and do not affect behaviour ctx->trace->pVtbl->EnableWriter(ctx->trace, AMF_TRACE_WRITER_DEBUG_OUTPUT, ctx->log_to_dbg != 0 ); @@ -187,6 +190,7 @@ static int amf_init_context(AVCodecContext *avctx) if (!ctx->hw_frames_ctx) { return AVERROR(ENOMEM); } + ctx->hwsurfaces_in_queue_max = device_ctx->initial_pool_size - 1; } else { if(res == AMF_NOT_SUPPORTED) av_log(avctx, AV_LOG_INFO, "avctx->hw_frames_ctx has D3D11 device which doesn't have D3D11VA interface, switching to default\n"); @@ -443,6 +447,75 @@ int ff_amf_encode_init(AVCodecContext *avctx) return ret; } +static AMF_RESULT amf_set_property_buffer(AMFSurface *object, const wchar_t *name, AMFBuffer *val) +{ + AMF_RESULT res; + AMFVariantStruct var; + res = AMFVariantInit(&var); + if (res == AMF_OK) { + AMFGuid guid_AMFInterface = IID_AMFInterface(); + AMFInterface *amf_interface; + res = val->pVtbl->QueryInterface(val, &guid_AMFInterface, (void**)&amf_interface); + + if (res == AMF_OK) { + res = AMFVariantAssignInterface(&var, amf_interface); + amf_interface->pVtbl->Release(amf_interface); + } + if (res == AMF_OK) { + res = object->pVtbl->SetProperty(object, name, var); + } + AMFVariantClear(&var); + } + return res; +} + +static AMF_RESULT amf_get_property_buffer(AMFData *object, const wchar_t *name, AMFBuffer **val) +{ + AMF_RESULT res; + AMFVariantStruct var; + res = AMFVariantInit(&var); + if (res == AMF_OK) { + res = object->pVtbl->GetProperty(object, name, &var); + if (res == AMF_OK) { + if (var.type == AMF_VARIANT_INTERFACE) { + AMFGuid guid_AMFBuffer = IID_AMFBuffer(); + AMFInterface *amf_interface = AMFVariantInterface(&var); + res = amf_interface->pVtbl->QueryInterface(amf_interface, &guid_AMFBuffer, (void**)val); + } else { + res = AMF_INVALID_DATA_TYPE; + } + } + AMFVariantClear(&var); + } + return res; +} + +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 +557,8 @@ 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 +571,14 @@ 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"); + + res = amf_set_property_buffer(surface, L"av_frame_ref", frame_ref_storage_buffer); + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "SetProperty failed for \"av_frame_ref\" with error %d\n", res); + ctx->hwsurfaces_in_queue++; + frame_ref_storage_buffer->pVtbl->Release(frame_ref_storage_buffer); } 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 +643,15 @@ 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; + res = amf_get_property_buffer(data, L"av_frame_ref", &frame_ref_storage_buffer); + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetProperty failed for \"av_frame_ref\" with error %d\n", res); + amf_release_buffer_with_frame_ref(frame_ref_storage_buffer); + ctx->hwsurfaces_in_queue--; + } + data->pVtbl->Release(data); AMF_RETURN_IF_FALSE(ctx, ret >= 0, ret, "amf_copy_buffer() failed with error %d\n", ret); @@ -589,7 +681,7 @@ int ff_amf_receive_packet(AVCodecContext *avctx, AVPacket *avpkt) av_log(avctx, AV_LOG_WARNING, "Data acquired but delayed drain submission got AMF_INPUT_FULL- should not happen\n"); } } - } else if (ctx->delayed_surface != NULL || ctx->delayed_drain || (ctx->eof && res_query != AMF_EOF)) { + } else if (ctx->delayed_surface != NULL || ctx->delayed_drain || (ctx->eof && res_query != AMF_EOF) || (ctx->hwsurfaces_in_queue >= ctx->hwsurfaces_in_queue_max)) { block_and_wait = 1; av_usleep(1000); // wait and poll again } diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h index 84f0aad2fa..0e4c6e4b89 100644 --- a/libavcodec/amfenc.h +++ b/libavcodec/amfenc.h @@ -62,6 +62,9 @@ typedef struct AmfContext { AVBufferRef *hw_device_ctx; ///< pointer to HW accelerator (decoder) AVBufferRef *hw_frames_ctx; ///< pointer to HW accelerator (frame allocator) + int hwsurfaces_in_queue; + int hwsurfaces_in_queue_max; + // helpers to handle async calls int delayed_drain; AMFSurface *delayed_surface; @@ -140,7 +143,7 @@ extern const enum AVPixelFormat ff_amf_pix_fmts[]; */ #define AMF_RETURN_IF_FALSE(avctx, exp, ret_value, /*message,*/ ...) \ if (!(exp)) { \ - av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \ + av_log(avctx, AV_LOG_FATAL, __VA_ARGS__); \ return ret_value; \ }