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