Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
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();
> +if (res == AMF_OK) {
> +AMFGuid guid_AMFInterface = IID_AMFInterface();
> +AMFInterface *amf_interface;
> +res = val->pVtbl->QueryInterface(val, _AMFInterface,
> (void**)_interface);
> +
> +if (res == AMF_OK) {
> +res = AMFVariantAssignInterface(, amf_interface);
> +amf_interface->pVtbl->Release(amf_interface);
> +}
> +if (res == AMF_OK) {
> +res = object->pVtbl->SetProperty(object, name, var);
> +}
> +AMFVariantClear();
> +}
> +return res;
> +}
> +
> +static AMF_RESULT amf_get_property_buffer(AMFData *object, const wchar_t
> *name, AMFBuffer **val)
> +{
> +AMF_RESULT res;
> +AMFVariantStruct var;
> +res = AMFVariantInit();
> +if (res == AMF_OK) {
> +res = object->pVtbl->GetProperty(object, name, );
> +if (res == AMF_OK) {
> +if (var.type == AMF_VARIANT_INTERFACE) {
> +AMFGuid guid_AMFBuffer = IID_AMFBuffer();
> +AMFInterface *amf_interface = AMFVariantInterface();
> +res = amf_interface->pVtbl->QueryInterface(amf_interface,
> _AMFBuffer, (void**)val);
> +} else {
> +res = AMF_INVALID_DATA_TYPE;
> +}
> +}
> +AMFVariantClear();
> +}
> +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), _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),
> _ref, sizeof(frame_ref));
> +
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
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 :).
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)
---
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;
} 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();
+if (res == AMF_OK) {
+AMFGuid guid_AMFInterface = IID_AMFInterface();
+AMFInterface *amf_interface;
+res = val->pVtbl->QueryInterface(val, _AMFInterface,
(void**)_interface);
+
+if (res == AMF_OK) {
+res = AMFVariantAssignInterface(, amf_interface);
+amf_interface->pVtbl->Release(amf_interface);
+}
+if (res == AMF_OK) {
+res = object->pVtbl->SetProperty(object, name, var);
+}
+AMFVariantClear();
+}
+return res;
+}
+
+static AMF_RESULT amf_get_property_buffer(AMFData *object, const wchar_t
*name, AMFBuffer **val)
+{
+AMF_RESULT res;
+AMFVariantStruct var;
+res = AMFVariantInit();
+if (res == AMF_OK) {
+res = object->pVtbl->GetProperty(object, name, );
+if (res == AMF_OK) {
+if (var.type == AMF_VARIANT_INTERFACE) {
+AMFGuid guid_AMFBuffer = IID_AMFBuffer();
+AMFInterface *amf_interface = AMFVariantInterface();
+res = amf_interface->pVtbl->QueryInterface(amf_interface,
_AMFBuffer, (void**)val);
+} else {
+res = AMF_INVALID_DATA_TYPE;
+}
+}
+AMFVariantClear();
+}
+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), _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),
_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(_frame_ref,
frame_ref_storage_buffer->pVtbl->GetNative(frame_ref_storage_buffer),
sizeof(av_frame_ref));
+av_frame_free(_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
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
On 08/04/18 20:13, Alexander Kravchenko wrote:
>
>
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>> Of Mark Thompson
>> Sent: Sunday, April 8, 2018 8:25 PM
>> 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
>>
>> On 06/04/18 11:25, Alexander Kravchenko wrote:
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 @ 0502d340] Static surface pool size
>> exceeded.
[mpeg2video @ 02f8d400] get_buffer() failed [mpeg2video @
02f8d400] thread_get_buffer() failed [mpeg2video @
02f8d400] get_buffer() failed (-12 ) Error
while decoding stream #0:1: Operation not permitted
>>>
>>> Hi, could you please review the following updated patch I added queue
>>> limit according initial pool size of incoming frame context.
>>> This solves the problem running this pipeline without -extra_hw_frames 16
>> option, but -extra_hw_frames option can be used to have bigger queue for
>> encoder.
>>
>> I think you've misunderstood /why/ the decoder has the pool size allocation
>> that it does. The decoder expects to use all of the surfaces it has
>> allocated in
>> the worst case - the difference between MPEG-2 and H.264 is that MPEG-2
>> can store at most two reference frames (the forward and backward
>> references for B-frames), while H.264 can store up to sixteen. Most H.264
>> streams don't use all sixteen references, but in theory they could (excepting
>> level restrictions, but they are generally quite iffy) so the decoder
>> allocates
>> space for all of those references even if they aren't used.
>>
>> I can believe that this patch happens to work if you have a simple stream
>> with limited references (streams rarely use more than two or three), but it
>> will certainly fail exactly as before for complex streams.
>>
>> If you want to hold onto more than one frame in the encoder then currently
>> you need to use the -extra_hw_frames option on the source (whether
>> decoder or filter) - that is exactly what it's there for. Some sort of
>> automatic
>> negotiation is suggested (there was some discussion on libav-devel a while
>> ago), but the requirement that it works through libavfilter is a difficult
>> one
>> with the current structure so nothing concrete is yet proposed. (That was
>> mostly considering libmfx, where it's even more of a problem because the
>> lookahead options can make the encoder queue over a hundred frames
>> internally.)
>>
>>> ---
>>> libavcodec/amfenc.c | 97
>>> -
>>> libavcodec/amfenc.h | 3 ++
>>> 2 files changed, 99 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c index
>>> 89a10ff253..eb7b20c252 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,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, _##InterfaceTypeTo,
>> (void**)); \
>>> +}
>>> +
>>> +#define AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, pThis, name, val) \
>>> +{ \
>>> +AMFInterface *amf_interface; \
>>> +AMFVariantStruct var; \
>>> +res = AMFVariantInit(); \
>>> +if (res == AMF_OK) { \
>>> +AMF_AV_QUERY_INTERFACE(res, val, AMFInterface,
>> amf_interface)\
>>> +if (res == AMF_OK) { \
>>> +res =
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Sunday, April 8, 2018 8:25 PM
> 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
>
> On 06/04/18 11:25, Alexander Kravchenko wrote:
> >>
> >> 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 @ 0502d340] Static surface pool size
> exceeded.
> >> [mpeg2video @ 02f8d400] get_buffer() failed [mpeg2video @
> >> 02f8d400] thread_get_buffer() failed [mpeg2video @
> >> 02f8d400] get_buffer() failed (-12 ) Error
> >> while decoding stream #0:1: Operation not permitted
> >
> > Hi, could you please review the following updated patch I added queue
> > limit according initial pool size of incoming frame context.
> > This solves the problem running this pipeline without -extra_hw_frames 16
> option, but -extra_hw_frames option can be used to have bigger queue for
> encoder.
>
> I think you've misunderstood /why/ the decoder has the pool size allocation
> that it does. The decoder expects to use all of the surfaces it has
> allocated in
> the worst case - the difference between MPEG-2 and H.264 is that MPEG-2
> can store at most two reference frames (the forward and backward
> references for B-frames), while H.264 can store up to sixteen. Most H.264
> streams don't use all sixteen references, but in theory they could (excepting
> level restrictions, but they are generally quite iffy) so the decoder
> allocates
> space for all of those references even if they aren't used.
>
> I can believe that this patch happens to work if you have a simple stream
> with limited references (streams rarely use more than two or three), but it
> will certainly fail exactly as before for complex streams.
>
> If you want to hold onto more than one frame in the encoder then currently
> you need to use the -extra_hw_frames option on the source (whether
> decoder or filter) - that is exactly what it's there for. Some sort of
> automatic
> negotiation is suggested (there was some discussion on libav-devel a while
> ago), but the requirement that it works through libavfilter is a difficult one
> with the current structure so nothing concrete is yet proposed. (That was
> mostly considering libmfx, where it's even more of a problem because the
> lookahead options can make the encoder queue over a hundred frames
> internally.)
>
> > ---
> > libavcodec/amfenc.c | 97
> > -
> > libavcodec/amfenc.h | 3 ++
> > 2 files changed, 99 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c index
> > 89a10ff253..eb7b20c252 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,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, _##InterfaceTypeTo,
> (void**)); \
> > +}
> > +
> > +#define AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, pThis, name, val) \
> > +{ \
> > +AMFInterface *amf_interface; \
> > +AMFVariantStruct var; \
> > +res = AMFVariantInit(); \
> > +if (res == AMF_OK) { \
> > +AMF_AV_QUERY_INTERFACE(res, val, AMFInterface,
> amf_interface)\
> > +if (res == AMF_OK) { \
> > +res = AMFVariantAssignInterface(, amf_interface); \
> > +amf_interface->pVtbl->Release(amf_interface); \
> >
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
On 06/04/18 11:25, Alexander Kravchenko wrote:
>>
>> 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 @ 0502d340] Static surface pool size exceeded.
>> [mpeg2video @ 02f8d400] get_buffer() failed
>> [mpeg2video @ 02f8d400] thread_get_buffer() failed
>> [mpeg2video @ 02f8d400] get_buffer() failed (-12 )
>> Error while decoding stream #0:1: Operation not permitted
>
> Hi, could you please review the following updated patch
> I added queue limit according initial pool size of incoming frame context.
> This solves the problem running this pipeline without -extra_hw_frames 16
> option, but -extra_hw_frames option can be used to have bigger queue for
> encoder.
I think you've misunderstood /why/ the decoder has the pool size allocation
that it does. The decoder expects to use all of the surfaces it has allocated
in the worst case - the difference between MPEG-2 and H.264 is that MPEG-2 can
store at most two reference frames (the forward and backward references for
B-frames), while H.264 can store up to sixteen. Most H.264 streams don't use
all sixteen references, but in theory they could (excepting level restrictions,
but they are generally quite iffy) so the decoder allocates space for all of
those references even if they aren't used.
I can believe that this patch happens to work if you have a simple stream with
limited references (streams rarely use more than two or three), but it will
certainly fail exactly as before for complex streams.
If you want to hold onto more than one frame in the encoder then currently you
need to use the -extra_hw_frames option on the source (whether decoder or
filter) - that is exactly what it's there for. Some sort of automatic
negotiation is suggested (there was some discussion on libav-devel a while
ago), but the requirement that it works through libavfilter is a difficult one
with the current structure so nothing concrete is yet proposed. (That was
mostly considering libmfx, where it's even more of a problem because the
lookahead options can make the encoder queue over a hundred frames internally.)
> ---
> libavcodec/amfenc.c | 97
> -
> libavcodec/amfenc.h | 3 ++
> 2 files changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 89a10ff253..eb7b20c252 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,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, _##InterfaceTypeTo,
> (void**)); \
> +}
> +
> +#define AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, pThis, name, val) \
> +{ \
> +AMFInterface *amf_interface; \
> +AMFVariantStruct var; \
> +res = AMFVariantInit(); \
> +if (res == AMF_OK) { \
> +AMF_AV_QUERY_INTERFACE(res, val, AMFInterface, amf_interface)\
> +if (res == AMF_OK) { \
> +res = AMFVariantAssignInterface(, amf_interface); \
> +amf_interface->pVtbl->Release(amf_interface); \
> +} \
> +if (res == AMF_OK) { \
> +res = pThis->pVtbl->SetProperty(pThis, name, var); \
> +} \
> +AMFVariantClear(); \
> +}\
> +}
> +
> +#define AMF_AV_GET_PROPERTY_INTERFACE(res, pThis, name, TargetType, val) \
> +{ \
> +AMFVariantStruct var; \
> +res = AMFVariantInit(); \
> +if (res == AMF_OK) { \
> +res = pThis->pVtbl->GetProperty(pThis, name, ); \
> +if
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
> >> 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 @ 0502d340] Static surface pool size exceeded. > >> [mpeg2video @ 02f8d400] get_buffer() failed > >> [mpeg2video @ 02f8d400] thread_get_buffer() failed > >> [mpeg2video @ 02f8d400] get_buffer() failed (-12 ) > >> Error while decoding stream #0:1: Operation not permitted > > > > Hi, could you please review the following updated patch > > I added queue limit according initial pool size of incoming frame context. > > This solves the problem running this pipeline without -extra_hw_frames 16 > > option, but -extra_hw_frames option can be used to > have bigger queue for encoder. > > Yes, this solves it, and the output does indeed look good now. Thanks. > > I'll leave reviewing this patch to someone more familiar with hw encoding. Thanks, James. I think the following approaches could help to solve issues with pool size more flexible way 1) communication between decoder and the following frame consumer component (filter or encoder) about extra frames requirement in hw frame pool. 2) add function like av_buffer_pool_test(AVBufferPool *pool); which returns current status of pool (used/free frames count). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
On 4/6/2018 7:25 AM, Alexander Kravchenko wrote: >> >> 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 @ 0502d340] Static surface pool size exceeded. >> [mpeg2video @ 02f8d400] get_buffer() failed >> [mpeg2video @ 02f8d400] thread_get_buffer() failed >> [mpeg2video @ 02f8d400] get_buffer() failed (-12 ) >> Error while decoding stream #0:1: Operation not permitted > > Hi, could you please review the following updated patch > I added queue limit according initial pool size of incoming frame context. > This solves the problem running this pipeline without -extra_hw_frames 16 > option, but -extra_hw_frames option can be used to have bigger queue for > encoder. Yes, this solves it, and the output does indeed look good now. Thanks. I'll leave reviewing this patch to someone more familiar with hw encoding. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
>
> 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 @ 0502d340] Static surface pool size exceeded.
> [mpeg2video @ 02f8d400] get_buffer() failed
> [mpeg2video @ 02f8d400] thread_get_buffer() failed
> [mpeg2video @ 02f8d400] get_buffer() failed (-12 )
> Error while decoding stream #0:1: Operation not permitted
Hi, could you please review the following updated patch
I added queue limit according initial pool size of incoming frame context.
This solves the problem running this pipeline without -extra_hw_frames 16
option, but -extra_hw_frames option can be used to have bigger queue for
encoder.
---
libavcodec/amfenc.c | 97 -
libavcodec/amfenc.h | 3 ++
2 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index 89a10ff253..eb7b20c252 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,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, _##InterfaceTypeTo,
(void**)); \
+}
+
+#define AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, pThis, name, val) \
+{ \
+AMFInterface *amf_interface; \
+AMFVariantStruct var; \
+res = AMFVariantInit(); \
+if (res == AMF_OK) { \
+AMF_AV_QUERY_INTERFACE(res, val, AMFInterface, amf_interface)\
+if (res == AMF_OK) { \
+res = AMFVariantAssignInterface(, amf_interface); \
+amf_interface->pVtbl->Release(amf_interface); \
+} \
+if (res == AMF_OK) { \
+res = pThis->pVtbl->SetProperty(pThis, name, var); \
+} \
+AMFVariantClear(); \
+}\
+}
+
+#define AMF_AV_GET_PROPERTY_INTERFACE(res, pThis, name, TargetType, val) \
+{ \
+AMFVariantStruct var; \
+res = AMFVariantInit(); \
+if (res == AMF_OK) { \
+res = pThis->pVtbl->GetProperty(pThis, name, ); \
+if (res == AMF_OK) { \
+if (var.type == AMF_VARIANT_INTERFACE &&
AMFVariantInterface()) { \
+AMF_AV_QUERY_INTERFACE(res, AMFVariantInterface(),
TargetType, val); \
+} else { \
+res = AMF_INVALID_DATA_TYPE; \
+} \
+} \
+AMFVariantClear(); \
+}\
+}
+
+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), _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),
_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(_frame_ref,
frame_ref_storage_buffer->pVtbl->GetNative(frame_ref_storage_buffer),
sizeof(av_frame_ref));
+av_frame_free(_frame_ref);
+frame_ref_storage_buffer->pVtbl->Release(frame_ref_storage_buffer);
+}
int ff_amf_send_frame(AVCodecContext *avctx, const AVFrame *frame)
{
@@ -484,6 +555,7 @@ int ff_amf_send_frame(AVCodecContext *avctx, const AVFrame
*frame)
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
> > 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 @ 0502d340] Static surface pool size exceeded. > [mpeg2video @ 02f8d400] get_buffer() failed > [mpeg2video @ 02f8d400] thread_get_buffer() failed > [mpeg2video @ 02f8d400] get_buffer() failed (-12 ) > Error while decoding stream #0:1: Operation not permitted Hi, I have checked the test, it causes such error because dxva decoder allocates small pool size for input of AV_CODEC_ID_MPEG2VIDEO Option "-extra_hw_frames 16" solves this problem. I have checked test using the following command line. Result video looks ok comparing to original video. ./ffmpeg -hwaccel d3d11va -hwaccel_output_format d3d11 -extra_hw_frames 12 -i matrixbench_mpeg2.mpg -an -c:v h264_amf out.mkv I have found the logic in dxva.c which sets initial pool size /* add surfaces based on number of possible refs */ if (avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id == AV_CODEC_ID_HEVC) num_surfaces += 16; else if (avctx->codec_id == AV_CODEC_ID_VP9) num_surfaces += 8; else num_surfaces += 2; ... frames_ctx->initial_pool_size = num_surfaces; There is alternative way to solve this problem, such as using copy surface before submitting to encoder, but this scenario will spend additional resources. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
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, _##InterfaceTypeTo,
> (void**)); \
> +}
> +
> +#define AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, pThis, name, val) \
> +{ \
> +AMFInterface *amf_interface; \
> +AMFVariantStruct var; \
> +res = AMFVariantInit(); \
> +if (res == AMF_OK) { \
> +AMF_AV_QUERY_INTERFACE(res, val, AMFInterface, amf_interface)\
> +if (res == AMF_OK) { \
> +res = AMFVariantAssignInterface(, amf_interface); \
> +amf_interface->pVtbl->Release(amf_interface); \
> +} \
> +if (res == AMF_OK) { \
> +res = pThis->pVtbl->SetProperty(pThis, name, var); \
> +} \
> +AMFVariantClear(); \
> +}\
> +}
> +
> +#define AMF_AV_GET_PROPERTY_INTERFACE(res, pThis, name, TargetType, val) \
> +{ \
> +AMFVariantStruct var; \
> +res = AMFVariantInit(); \
> +if (res == AMF_OK) { \
> +res = pThis->pVtbl->GetProperty(pThis, name, ); \
> +if (res == AMF_OK) { \
> +if (var.type == AMF_VARIANT_INTERFACE &&
> AMFVariantInterface()) { \
> +AMF_AV_QUERY_INTERFACE(res, AMFVariantInterface(),
> TargetType, val); \
> +} else { \
> +res = AMF_INVALID_DATA_TYPE; \
> +} \
> +} \
> +AMFVariantClear(); \
> +}\
> +}
> +
> +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), _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),
> _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(_frame_ref,
> frame_ref_storage_buffer->pVtbl->GetNative(frame_ref_storage_buffer),
> sizeof(av_frame_ref));
> +av_frame_free(_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,
>
[FFmpeg-devel] [PATCH] lavc/amfenc: Retain a reference to D3D frames used as input during the encoding process
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, _##InterfaceTypeTo,
(void**)); \
+}
+
+#define AMF_AV_ASSIGN_PROPERTY_INTERFACE(res, pThis, name, val) \
+{ \
+AMFInterface *amf_interface; \
+AMFVariantStruct var; \
+res = AMFVariantInit(); \
+if (res == AMF_OK) { \
+AMF_AV_QUERY_INTERFACE(res, val, AMFInterface, amf_interface)\
+if (res == AMF_OK) { \
+res = AMFVariantAssignInterface(, amf_interface); \
+amf_interface->pVtbl->Release(amf_interface); \
+} \
+if (res == AMF_OK) { \
+res = pThis->pVtbl->SetProperty(pThis, name, var); \
+} \
+AMFVariantClear(); \
+}\
+}
+
+#define AMF_AV_GET_PROPERTY_INTERFACE(res, pThis, name, TargetType, val) \
+{ \
+AMFVariantStruct var; \
+res = AMFVariantInit(); \
+if (res == AMF_OK) { \
+res = pThis->pVtbl->GetProperty(pThis, name, ); \
+if (res == AMF_OK) { \
+if (var.type == AMF_VARIANT_INTERFACE &&
AMFVariantInterface()) { \
+AMF_AV_QUERY_INTERFACE(res, AMFVariantInterface(),
TargetType, val); \
+} else { \
+res = AMF_INVALID_DATA_TYPE; \
+} \
+} \
+AMFVariantClear(); \
+}\
+}
+
+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), _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),
_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(_frame_ref,
frame_ref_storage_buffer->pVtbl->GetNative(frame_ref_storage_buffer),
sizeof(av_frame_ref));
+av_frame_free(_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,
