[FFmpeg-devel] [PATCH] avutil/hwcontext_cuda: add AVCUDAFramesContext and AVCUDAFramesContext.flags

[Timo Rothenpieler]

Am 07.05.2018 um 23:22 schrieb Mark Thompson:
> On 07/05/18 22:10, Timo Rothenpieler wrote:
>> Frames can be mapped from nvdec/cuvid, not needing any actual memory
>> allocation, but all other features of the hw_frames_ctx.
>> Hence the dummy-mode, which does not allocate any (notable amounts of)
>> memory but otherwise behaves the exact same.
>> ---
>>   doc/APIchanges             |  3 +++
>>   libavutil/hwcontext_cuda.c | 10 ++++++++++
>>   libavutil/hwcontext_cuda.h | 18 +++++++++++++++++-
>>   libavutil/version.h        |  2 +-
>>   4 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index ede5b186ae..82ec888fd8 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>>   
>>   API changes, most recent first:
>>   
>> +2018-05-xx - xxxxxxxxxx - lavu 56.19.100 - hwcontext.h
>> +  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
>> +
>>   2018-04-xx - xxxxxxxxxx - lavu 56.18.100 - pixdesc.h
>>     Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8.
>>   
>> diff --git a/libavutil/hwcontext_cuda.c b/libavutil/hwcontext_cuda.c
>> index 37827a770c..0d867ef0f5 100644
>> --- a/libavutil/hwcontext_cuda.c
>> +++ b/libavutil/hwcontext_cuda.c
>> @@ -83,6 +83,7 @@ static void cuda_buffer_free(void *opaque, uint8_t *data)
>>   static AVBufferRef *cuda_pool_alloc(void *opaque, int size)
>>   {
>>       AVHWFramesContext     *ctx = opaque;
>> +    AVCUDAFramesContext *frctx = ctx->hwctx;
>>       AVCUDADeviceContext *hwctx = ctx->device_ctx->hwctx;
>>       CudaFunctions          *cu = hwctx->internal->cuda_dl;
>>   
>> @@ -97,6 +98,10 @@ static AVBufferRef *cuda_pool_alloc(void *opaque, int size)
>>           return NULL;
>>       }
>>   
>> +    // A lot of places expect the pointer to be !=NULL, so make minimum allocation instead.
>> +    if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
>> +        size = 1;
> 
> What are those places - can we get rid of them instead?  Allocating a single byte here is rather yucky.

That'd be AVBuffer/Ref itself interpreting its alloc function returning 
NULL as ENOMEM. Doubt changing it is a good idea. And as any other 
return value than NULL indicates a valid pointer, this approach is what 
I picked.

>> +
>>       err = cu->cuMemAlloc(&data, size);
>>       if (err != CUDA_SUCCESS)
>>           goto fail;
>> @@ -161,6 +166,7 @@ static int cuda_frames_init(AVHWFramesContext *ctx)
>>   
>>   static int cuda_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
>>   {
>> +    AVCUDAFramesContext *frctx = ctx->hwctx;
>>       int aligned_width;
>>       int width_in_bytes = ctx->width;
>>   
>> @@ -210,6 +216,9 @@ static int cuda_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
>>       frame->width  = ctx->width;
>>       frame->height = ctx->height;
>>   
>> +    if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
>> +        frame->data[0] = frame->data[1] = frame->data[2] = NULL;
> 
> Are you intending linesize and buf[0] to have been filled with specific values here?
> 
> A comment saying exactly what is set and what isn't might help.  (Maybe the comment goes with the flag itself.)

Yes, that's intended. I'm only explicitly cleaning out those as they're 
dangerous to have pointing to buffers that are smaller than the rest of 
the frame advertises.

Describing this fact on the flag is probably a good idea.

>> +
>>       return 0;
>>   }
>>   
>> @@ -402,6 +411,7 @@ const HWContextType ff_hwcontext_type_cuda = {
>>       .name                 = "CUDA",
>>   
>>       .device_hwctx_size    = sizeof(AVCUDADeviceContext),
>> +    .frames_hwctx_size    = sizeof(AVCUDAFramesContext),
>>       .frames_priv_size     = sizeof(CUDAFramesContext),
>>   
>>       .device_create        = cuda_device_create,
>> diff --git a/libavutil/hwcontext_cuda.h b/libavutil/hwcontext_cuda.h
>> index 12dae8449e..388d6f8f1c 100644
>> --- a/libavutil/hwcontext_cuda.h
>> +++ b/libavutil/hwcontext_cuda.h
>> @@ -45,7 +45,23 @@ typedef struct AVCUDADeviceContext {
>>   } AVCUDADeviceContext;
>>   
>>   /**
>> - * AVHWFramesContext.hwctx is currently not used
>> + * This struct is allocated as AVHWFramesContext.hwctx
>>    */
>> +typedef struct AVCUDAFramesContext {
>> +    /**
>> +     * Special implementation-specific flags.
>> +     *
>> +     * Must be set by the user before calling av_hwframe_ctx_init().
>> +     */
> 
> This makes it sound like you /must/ write to the field.  Leaving it as zero is also fine.

Simply replacing "Must" with "May"?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3994 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180507/4a52f10b/attachment.bin>