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

2018-05-08 Thread Timo Rothenpieler 

Am 08.05.2018 um 17:49 schrieb wm4:

On Tue, 8 May 2018 17:43:49 +0200
Timo Rothenpieler  wrote:


-frame->buf[0] = av_buffer_pool_get(ctx->pool);
+if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
+frame->buf[0] = av_buffer_create(NULL, 0, NULL, NULL, 0);
+else
+frame->buf[0] = av_buffer_pool_get(ctx->pool);
+


Is this really needed? Because at least videotoolbox also lets the
decoder allocate frames, and allocates the "dummy" buffers outside of
the hwcontext. (I don't quite remember how it works.)


You mean compared to just leaving buf[0] empty?



No, compared to how the videotoolbox code does things.


videotoolbox seems to use an externally supplied AVHWFramesContext::pool.
Which should be possible to do for cuda as well, i.e. a custom buffer 
pool that returns mostly empty data, so the new API would be unneeded. 
I'll have a look if it works out.




smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-08 Thread wm4 
On Tue, 8 May 2018 17:43:49 +0200
Timo Rothenpieler  wrote:

> >> -frame->buf[0] = av_buffer_pool_get(ctx->pool);
> >> +if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
> >> +frame->buf[0] = av_buffer_create(NULL, 0, NULL, NULL, 0);
> >> +else
> >> +frame->buf[0] = av_buffer_pool_get(ctx->pool);
> >> +  
> > 
> > Is this really needed? Because at least videotoolbox also lets the
> > decoder allocate frames, and allocates the "dummy" buffers outside of
> > the hwcontext. (I don't quite remember how it works.)  
> 
> You mean compared to just leaving buf[0] empty?
> 

No, compared to how the videotoolbox code does things.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-08 Thread Timo Rothenpieler 

-frame->buf[0] = av_buffer_pool_get(ctx->pool);
+if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
+frame->buf[0] = av_buffer_create(NULL, 0, NULL, NULL, 0);
+else
+frame->buf[0] = av_buffer_pool_get(ctx->pool);
+


Is this really needed? Because at least videotoolbox also lets the
decoder allocate frames, and allocates the "dummy" buffers outside of
the hwcontext. (I don't quite remember how it works.)


You mean compared to just leaving buf[0] empty?



smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-08 Thread wm4 
On Mon,  7 May 2018 23:46:48 +0200
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 | 12 +++-
>  libavutil/hwcontext_cuda.h | 22 +-
>  libavutil/version.h|  2 +-
>  4 files changed, 36 insertions(+), 3 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 - xx - lavu 56.19.100 - hwcontext.h
> +  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
> +
>  2018-04-xx - xx - 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..b0b4bf24ae 100644
> --- a/libavutil/hwcontext_cuda.c
> +++ b/libavutil/hwcontext_cuda.c
> @@ -161,6 +161,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;
>  
> @@ -171,7 +172,11 @@ static int cuda_get_buffer(AVHWFramesContext *ctx, 
> AVFrame *frame)
>  }
>  aligned_width = FFALIGN(width_in_bytes, CUDA_FRAME_ALIGNMENT);
>  
> -frame->buf[0] = av_buffer_pool_get(ctx->pool);
> +if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
> +frame->buf[0] = av_buffer_create(NULL, 0, NULL, NULL, 0);
> +else
> +frame->buf[0] = av_buffer_pool_get(ctx->pool);
> +

Is this really needed? Because at least videotoolbox also lets the
decoder allocate frames, and allocates the "dummy" buffers outside of
the hwcontext. (I don't quite remember how it works.)

>  if (!frame->buf[0])
>  return AVERROR(ENOMEM);
>  
> @@ -210,6 +215,10 @@ static int cuda_get_buffer(AVHWFramesContext *ctx, 
> AVFrame *frame)
>  frame->width  = ctx->width;
>  frame->height = ctx->height;
>  
> +// they're pointing to invalid memory, dangerous to leave set
> +if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
> +frame->data[0] = frame->data[1] = frame->data[2] = NULL;
> +
>  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..19accbd9be 100644
> --- a/libavutil/hwcontext_cuda.h
> +++ b/libavutil/hwcontext_cuda.h
> @@ -45,7 +45,27 @@ typedef struct AVCUDADeviceContext {
>  } AVCUDADeviceContext;
>  
>  /**
> - * AVHWFramesContext.hwctx is currently not used
> + * This struct is allocated as AVHWFramesContext.hwctx
>   */
> +typedef struct AVCUDAFramesContext {
> +/**
> + * Special implementation-specific flags.
> + *
> + * May be set by the user before calling av_hwframe_ctx_init().
> + */
> +int flags;
> +} AVCUDAFramesContext;
> +
> +/**
> + * No actual allocation will happen, but otherwise behaves like normal.
> + *
> + * This is to be used if a AVHWFramesContext is required, but the actual
> + * allocation is happening outside of it.
> + *
> + * The resulting AVFrames will be identical to normal frames, except for
> + * their data[] pointers being NULL and the AVBufferRef in buf[0] being
> + * set but containing no notable allocation of memory.
> + */
> +#define AV_CUDA_HWFRAMES_DUMMY_MODE (1 << 0)
>  
>  #endif /* AVUTIL_HWCONTEXT_CUDA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 5185454d9b..84409b1d69 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  56
> -#define LIBAVUTIL_VERSION_MINOR  18
> +#define LIBAVUTIL_VERSION_MINOR  19
>  #define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-08 Thread Daniel Oberhoff 
> 
>> 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.

Can someone explain the actual use-case? We do direct mapping of cuvid/nvdec 
frames to transfer them to/from gl, but i have no idea how this is related to 
that...



signature.asc
Description: Message signed with OpenPGP
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-07 Thread Wang Bin 
2018-05-08 5:10 GMT+08:00 Timo Rothenpieler :

> 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 - xx - lavu 56.19.100 - hwcontext.h
> +  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
> +
>  2018-04-xx - xx - 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;
> +
>  err = cu->cuMemAlloc(, 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;
> +
>  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().
> + */
> +int flags;
> +} AVCUDAFramesContext;
> +
> +/**
> + * No actual allocation will happen, but otherwise behaves like normal.
> + *
> + * This is to be used if a AVHWFramesContext is required, but the actual
> + * allocation has to happen outside of it.
> + */
> +#define AV_CUDA_HWFRAMES_DUMMY_MODE (1 << 0)
>
>  #endif /* AVUTIL_HWCONTEXT_CUDA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 5185454d9b..84409b1d69 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>
>  #define LIBAVUTIL_VERSION_MAJOR  56
> -#define LIBAVUTIL_VERSION_MINOR  18
> +#define LIBAVUTIL_VERSION_MINOR  19
>  #define LIBAVUTIL_VERSION_MICRO 100
>
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> --
>

I think the flag should be added in avcodec, maybe AVCodecContext.flags2,
becuase
1. This flag is used by cuda decoder only, but not works avfilter and other
cuda apis.
2. Other hw codecs may support referencing the decoded memory instead of
copying it, for example, mmal decoder. see
https://github.com/wang-bin/FFmpeg/commit/74390ec836743dd337c92dd5da5f4f9ff638316b#diff-26a23a40f16babe0068c7340c6160472
3. the flag can be turned on/off at any time
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-07 Thread Mark Thompson 
On 07/05/18 22:28, Timo Rothenpieler wrote:
> 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 - xx - lavu 56.19.100 - hwcontext.h
>>> +  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
>>> +
>>>   2018-04-xx - xx - 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.

You could make a new function called, say, "cuda_pool_dummy_alloc" which always 
returns a non-null pointer (to some random static variable).  Then set that as 
the allocation function for the buffer pool in dummy mode and drop this 
changing of size.  (That seems cleaner?  Or maybe I'm overthinking this...)

>>> +
>>>   err = cu->cuMemAlloc(, 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.

Ok, sounds good.

>>> +
>>>   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"?

Sure.  Looking

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

2018-05-07 Thread Timo Rothenpieler 
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 | 12 +++-
 libavutil/hwcontext_cuda.h | 22 +-
 libavutil/version.h|  2 +-
 4 files changed, 36 insertions(+), 3 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 - xx - lavu 56.19.100 - hwcontext.h
+  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
+
 2018-04-xx - xx - 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..b0b4bf24ae 100644
--- a/libavutil/hwcontext_cuda.c
+++ b/libavutil/hwcontext_cuda.c
@@ -161,6 +161,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;
 
@@ -171,7 +172,11 @@ static int cuda_get_buffer(AVHWFramesContext *ctx, AVFrame 
*frame)
 }
 aligned_width = FFALIGN(width_in_bytes, CUDA_FRAME_ALIGNMENT);
 
-frame->buf[0] = av_buffer_pool_get(ctx->pool);
+if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
+frame->buf[0] = av_buffer_create(NULL, 0, NULL, NULL, 0);
+else
+frame->buf[0] = av_buffer_pool_get(ctx->pool);
+
 if (!frame->buf[0])
 return AVERROR(ENOMEM);
 
@@ -210,6 +215,10 @@ static int cuda_get_buffer(AVHWFramesContext *ctx, AVFrame 
*frame)
 frame->width  = ctx->width;
 frame->height = ctx->height;
 
+// they're pointing to invalid memory, dangerous to leave set
+if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
+frame->data[0] = frame->data[1] = frame->data[2] = NULL;
+
 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..19accbd9be 100644
--- a/libavutil/hwcontext_cuda.h
+++ b/libavutil/hwcontext_cuda.h
@@ -45,7 +45,27 @@ typedef struct AVCUDADeviceContext {
 } AVCUDADeviceContext;
 
 /**
- * AVHWFramesContext.hwctx is currently not used
+ * This struct is allocated as AVHWFramesContext.hwctx
  */
+typedef struct AVCUDAFramesContext {
+/**
+ * Special implementation-specific flags.
+ *
+ * May be set by the user before calling av_hwframe_ctx_init().
+ */
+int flags;
+} AVCUDAFramesContext;
+
+/**
+ * No actual allocation will happen, but otherwise behaves like normal.
+ *
+ * This is to be used if a AVHWFramesContext is required, but the actual
+ * allocation is happening outside of it.
+ *
+ * The resulting AVFrames will be identical to normal frames, except for
+ * their data[] pointers being NULL and the AVBufferRef in buf[0] being
+ * set but containing no notable allocation of memory.
+ */
+#define AV_CUDA_HWFRAMES_DUMMY_MODE (1 << 0)
 
 #endif /* AVUTIL_HWCONTEXT_CUDA_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 5185454d9b..84409b1d69 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  18
+#define LIBAVUTIL_VERSION_MINOR  19
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-07 Thread 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 - xx - lavu 56.19.100 - hwcontext.h

+  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
+
  2018-04-xx - xx - 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(, 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"?



smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-07 Thread 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 - xx - lavu 56.19.100 - hwcontext.h
> +  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
> +
>  2018-04-xx - xx - 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.

> +
>  err = cu->cuMemAlloc(, 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.)

> +
>  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.

> +int flags;
> +} AVCUDAFramesContext;
> +
> +/**
> + * No actual allocation will happen, but otherwise behaves like normal.
> + *
> + * This is to be used if a AVHWFramesContext is required, but the actual
> + * allocation has to happen outside of it.
> + */
> +#define AV_CUDA_HWFRAMES_DUMMY_MODE (1 << 0)
>  
>  #endif /* AVUTIL_HWCONTEXT_CUDA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 5185454d9b..84409b1d69 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  56
> -#define LIBAVUTIL_VERSION_MINOR  18
> +#define LIBAVUTIL_VERSION_MINOR  19
>  #define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> 

This approach seems ok to me.

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-05-07 Thread Timo Rothenpieler 
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 - xx - lavu 56.19.100 - hwcontext.h
+  Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
+
 2018-04-xx - xx - 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;
+
 err = cu->cuMemAlloc(, 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;
+
 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().
+ */
+int flags;
+} AVCUDAFramesContext;
+
+/**
+ * No actual allocation will happen, but otherwise behaves like normal.
+ *
+ * This is to be used if a AVHWFramesContext is required, but the actual
+ * allocation has to happen outside of it.
+ */
+#define AV_CUDA_HWFRAMES_DUMMY_MODE (1 << 0)
 
 #endif /* AVUTIL_HWCONTEXT_CUDA_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 5185454d9b..84409b1d69 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  18
+#define LIBAVUTIL_VERSION_MINOR  19
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel