Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Diego Biurrun]

On Thu, Jun 29, 2017 at 10:32:16AM +0100, Mark Thompson wrote:
> On 29/06/17 08:31, Diego Biurrun wrote:
> > On Sun, Jun 18, 2017 at 07:08:02PM +0100, Mark Thompson wrote:
> >> --- /dev/null
> >> +++ b/libavutil/hwcontext_drm.h
> >> @@ -0,0 +1,145 @@
> >> +
> >> +#ifndef AVUTIL_HWCONTEXT_DRM_H
> >> +#define AVUTIL_HWCONTEXT_DRM_H
> >> +
> >> +#include "frame.h"
> > 
> > frame.h is not used,
> 
> AV_NUM_DATA_POINTERS
> 
> (Though there were thoughts that we could make this a new constant inside the 
> header, probably with the value 4.  Still undecided on that one.)
> 
> >  but stdint.h and sys/types.h are.
> 
> stddef.h and stdint.h are included by frame.h.  I don't see why sys/types.h 
> would be needed?

For size_t, but that's stddef.h, so you're right.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Mark Thompson]

On 29/06/17 08:31, Diego Biurrun wrote:
> On Sun, Jun 18, 2017 at 07:08:02PM +0100, Mark Thompson wrote:
>> --- a/configure
>> +++ b/configure
>> @@ -190,6 +190,7 @@ External library support:
>>--enable-avisynth  video frameserver
>>--enable-avxsynth  Linux version of AviSynth
>>--enable-bzlib bzip2 compression [autodetect]
>> +  --enable-drm   DRM buffer sharing
>>--enable-frei0rvideo filtering plugins
>>--enable-gnutlscrypto
>>--enable-libbs2b   Bauer stereophonic-to-binaural DSP
>> @@ -1303,6 +1304,7 @@ EXTERNAL_LIBRARY_LIST="
>>  $EXTERNAL_LIBRARY_VERSION3_LIST
>>  avisynth
>>  avxsynth
>> +drm
>>  frei0r
>>  gnutls
>>  libbs2b
> 
> I think "libdrm" would be a better name for the component. AFAIU it is
> the actual name of the library.

Sure; will change.

>> @@ -4734,6 +4736,7 @@ done
>>  enabled avisynth  && require_header avisynth/avisynth_c.h
>>  enabled avxsynth  && require_header avxsynth/avxsynth_c.h
>>  enabled cuda  && require cuda cuda.h cuInit -lcuda
>> +enabled drm   && require_pkg_config libdrm libdrm xf86drm.h 
>> drmGetVersion
> 
> The first argument should be the build-system-internal name of the component,
> this will not work quite as expected. See what I wrote above.

Ok.

>> --- /dev/null
>> +++ b/libavutil/hwcontext_drm.c
>> @@ -0,0 +1,281 @@
>> --- /dev/null
>> +++ b/libavutil/hwcontext_drm.h
>> @@ -0,0 +1,145 @@
>> +
>> +#ifndef AVUTIL_HWCONTEXT_DRM_H
>> +#define AVUTIL_HWCONTEXT_DRM_H
>> +
>> +#include "frame.h"
> 
> frame.h is not used,

AV_NUM_DATA_POINTERS

(Though there were thoughts that we could make this a new constant inside the 
header, probably with the value 4.  Still undecided on that one.)

>  but stdint.h and sys/types.h are.

stddef.h and stdint.h are included by frame.h.  I don't see why sys/types.h 
would be needed?

> I think you also need to bump libavutil minor version.

Yes.  (Omitted for mergability while still seeking comments on the structure.)

Thanks,

- Mark

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Diego Biurrun]

On Sun, Jun 18, 2017 at 07:08:02PM +0100, Mark Thompson wrote:
> --- a/configure
> +++ b/configure
> @@ -190,6 +190,7 @@ External library support:
>--enable-avisynth  video frameserver
>--enable-avxsynth  Linux version of AviSynth
>--enable-bzlib bzip2 compression [autodetect]
> +  --enable-drm   DRM buffer sharing
>--enable-frei0rvideo filtering plugins
>--enable-gnutlscrypto
>--enable-libbs2b   Bauer stereophonic-to-binaural DSP
> @@ -1303,6 +1304,7 @@ EXTERNAL_LIBRARY_LIST="
>  $EXTERNAL_LIBRARY_VERSION3_LIST
>  avisynth
>  avxsynth
> +drm
>  frei0r
>  gnutls
>  libbs2b

I think "libdrm" would be a better name for the component. AFAIU it is
the actual name of the library.

> @@ -4734,6 +4736,7 @@ done
>  enabled avisynth  && require_header avisynth/avisynth_c.h
>  enabled avxsynth  && require_header avxsynth/avxsynth_c.h
>  enabled cuda  && require cuda cuda.h cuInit -lcuda
> +enabled drm   && require_pkg_config libdrm libdrm xf86drm.h 
> drmGetVersion

The first argument should be the build-system-internal name of the component,
this will not work quite as expected. See what I wrote above.

> --- /dev/null
> +++ b/libavutil/hwcontext_drm.c
> @@ -0,0 +1,281 @@
> --- /dev/null
> +++ b/libavutil/hwcontext_drm.h
> @@ -0,0 +1,145 @@
> +
> +#ifndef AVUTIL_HWCONTEXT_DRM_H
> +#define AVUTIL_HWCONTEXT_DRM_H
> +
> +#include "frame.h"

frame.h is not used, but stdint.h and sys/types.h are.

I think you also need to bump libavutil minor version.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Mark Thompson]

On 28/06/17 16:24, Rémi Denis-Courmont wrote:
> Le sunnuntaina 18. kesäkuuta 2017, 19.08.02 EEST Mark Thompson a écrit :
>> ---
>> The intent of this is to have a common structure which can be used in all
>> cases where DRM objects need to be shared between components.  It would be
>> helpful if anyone familiar with specific drivers or use-cases could ensure
>> that the structure (see the hwcontext_drm.h header) is sufficiently general
>> to cover them - we would like this to the one answer and never require any
>> more formats in future.
> 
> AFAIU, the main point of using DRM directly is abstraction from the windowing 
> system. So the dependency on Xf68 looks very suspicious. It should work with 
> X11, with Wayland, with plain KMS or even with just a render node (e.g. 
> transcoding).
> 
> I suspect that this may be tying X11-DRI and kernel DRM too tightly.

There is no X11 dependency; the libdrm header has xf86 in the name for 
hysterical raisins.



___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[wm4]

On Wed, 28 Jun 2017 11:37:57 +0200
Luca Barbato  wrote:

> On 6/28/17 11:36 AM, wm4 wrote:
> > Or maybe we should get back to your old nested arrays, but I'm worried
> > about excessive memory usage.  
> 
> The alternative is malloc it with all the usability issues around it =/

Or maybe limit them to 4 or whatever. Maybe I'm overthinking and
overcomplicating this.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[Luca Barbato]

On 6/28/17 11:36 AM, wm4 wrote:
> Or maybe we should get back to your old nested arrays, but I'm worried
> about excessive memory usage.

The alternative is malloc it with all the usability issues around it =/

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavu: Add DRM hwcontext

[wm4]

On Sun, 18 Jun 2017 19:08:02 +0100
Mark Thompson  wrote:

> ---
> The intent of this is to have a common structure which can be used in all 
> cases where DRM objects need to be shared between components.  It would be 
> helpful if anyone familiar with specific drivers or use-cases could ensure 
> that the structure (see the hwcontext_drm.h header) is sufficiently general 
> to cover them - we would like this to the one answer and never require any 
> more formats in future.
> 
> Also unclear about how the libdrm dependency should actually be included.  
> Alot of this (including the header) doesn't actually require it, and it 
> wouldn't be difficult to make it a build-time-only dependency by replacing a 
> few calls with ioctls().
> 
> (See  
> for more context.)
> 
> 
>  configure  |   5 +-
>  libavutil/Makefile |   1 +
>  libavutil/hwcontext.c  |   4 +
>  libavutil/hwcontext.h  |   1 +
>  libavutil/hwcontext_drm.c  | 281 
> +
>  libavutil/hwcontext_drm.h  | 145 +
>  libavutil/hwcontext_internal.h |   1 +
>  libavutil/pixdesc.c|   4 +
>  libavutil/pixfmt.h |   6 +
>  9 files changed, 447 insertions(+), 1 deletion(-)
>  create mode 100644 libavutil/hwcontext_drm.c
>  create mode 100644 libavutil/hwcontext_drm.h
> 
> diff --git a/configure b/configure
> index fd879bc9d..12d831995 100755
> --- a/configure
> +++ b/configure
> @@ -190,6 +190,7 @@ External library support:
>--enable-avisynth  video frameserver
>--enable-avxsynth  Linux version of AviSynth
>--enable-bzlib bzip2 compression [autodetect]
> +  --enable-drm   DRM buffer sharing
>--enable-frei0rvideo filtering plugins
>--enable-gnutlscrypto
>--enable-libbs2b   Bauer stereophonic-to-binaural DSP
> @@ -1303,6 +1304,7 @@ EXTERNAL_LIBRARY_LIST="
>  $EXTERNAL_LIBRARY_VERSION3_LIST
>  avisynth
>  avxsynth
> +drm
>  frei0r
>  gnutls
>  libbs2b
> @@ -2547,7 +2549,7 @@ avdevice_extralibs="libm_extralibs"
>  avformat_extralibs="libm_extralibs"
>  avfilter_extralibs="pthreads_extralibs libm_extralibs"
>  avresample_extralibs="libm_extralibs"
> -avutil_extralibs="clock_gettime_extralibs cuda_extralibs libm_extralibs 
> libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs 
> vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs 
> wincrypt_extralibs"
> +avutil_extralibs="clock_gettime_extralibs cuda_extralibs libdrm_extralibs 
> libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs 
> user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs 
> vdpau_x11_extralibs wincrypt_extralibs"
>  swscale_extralibs="libm_extralibs"
>  
>  # programs
> @@ -4734,6 +4736,7 @@ done
>  enabled avisynth  && require_header avisynth/avisynth_c.h
>  enabled avxsynth  && require_header avxsynth/avxsynth_c.h
>  enabled cuda  && require cuda cuda.h cuInit -lcuda
> +enabled drm   && require_pkg_config libdrm libdrm xf86drm.h 
> drmGetVersion
>  enabled frei0r&& require_header frei0r.h
>  enabled gnutls&& require_pkg_config gnutls gnutls 
> gnutls/gnutls.h gnutls_global_init
>  enabled libbs2b   && require_pkg_config libbs2b libbs2b bs2b.h 
> bs2b_open
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 6fb24db67..9493a0059 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -114,6 +114,7 @@ OBJS = adler32.o  
>   \
>  
>  OBJS-$(CONFIG_CUDA) += hwcontext_cuda.o
>  OBJS-$(CONFIG_D3D11VA)  += hwcontext_d3d11va.o
> +OBJS-$(CONFIG_DRM)  += hwcontext_drm.o
>  OBJS-$(CONFIG_DXVA2)+= hwcontext_dxva2.o
>  OBJS-$(CONFIG_LIBMFX)   += hwcontext_qsv.o
>  OBJS-$(CONFIG_LZO)  += lzo.o
> diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c
> index 6dc95bba1..e0febaf26 100644
> --- a/libavutil/hwcontext.c
> +++ b/libavutil/hwcontext.c
> @@ -35,6 +35,9 @@ static const HWContextType * const hw_table[] = {
>  #if CONFIG_D3D11VA
>  _hwcontext_type_d3d11va,
>  #endif
> +#if CONFIG_DRM
> +_hwcontext_type_drm,
> +#endif
>  #if CONFIG_DXVA2
>  _hwcontext_type_dxva2,
>  #endif
> @@ -52,6 +55,7 @@ static const HWContextType * const hw_table[] = {
>  
>  static const char *const hw_type_names[] = {
>  [AV_HWDEVICE_TYPE_CUDA]   = "cuda",
> +[AV_HWDEVICE_TYPE_DRM]= "drm",
>  [AV_HWDEVICE_TYPE_DXVA2]  = "dxva2",
>  [AV_HWDEVICE_TYPE_D3D11VA] = "d3d11va",
>  [AV_HWDEVICE_TYPE_QSV]= "qsv",
> diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
> index 203ea510e..a48b2e53c 100644
> ---