On Mon, 20 Feb 2017 23:29:20 +0000
Mark Thompson <[email protected]> wrote:

> On 20/02/17 06:52, wm4 wrote:
> > On Sun, 19 Feb 2017 18:46:42 +0000
> > Mark Thompson <[email protected]> wrote:
> >   
> >> Uses the cl_intel_va_api_media_sharing extension, which supports only
> >> NV12 surfaces and only mapping from QSV to OpenCL.  
> > 
> > No P010?  
> 
> Nope, NV12 only: 
> <https://www.khronos.org/registry/OpenCL/extensions/intel/cl_intel_va_api_media_sharing.txt>.
> 
> (Has anyone else ever wanted to do it?  Beignet didn't immediately work, I 
> had to send them 
> <https://cgit.freedesktop.org/beignet/commit/?id=3c31216a4ef148ba2e6048e30c62a7a7209407cb>.)

Well, you plan to use OpenCL for generic processing, and that wouldn't
work with 10 bit decoding. Unless you use the vaapi scaler or so to
convert to nv12 explicitly. (Which vf_scale_vaapi.c apparently can do
if it's told to.)

But it's nice to see that it can be easily extended, apparently.

> >> +static int opencl_map_from_qsv(AVHWFramesContext *dst_fc, AVFrame *dst,
> >> +                               const AVFrame *src, int flags)
> >> +{
> >> +    AVHWFramesContext      *src_fc =
> >> +        (AVHWFramesContext*)src->hw_frames_ctx->data;
> >> +    AVOpenCLDeviceContext *dst_dev = dst_fc->device_ctx->hwctx;
> >> +    OpenCLDeviceContext       *ctx = dst_fc->device_ctx->internal->priv;
> >> +    QSVtoOpenCLMapping    *mapping = NULL;
> >> +    VASurfaceID va_surface;
> >> +    cl_mem_flags cl_flags;
> >> +    cl_event event;
> >> +    cl_int cle;
> >> +    int err, i;
> >> +
> >> +    if (src->format == AV_PIX_FMT_QSV) {
> >> +        mfxFrameSurface1 *mfx_surface = (mfxFrameSurface1*)src->data[3];
> >> +        va_surface = *(VASurfaceID*)mfx_surface->Data.MemId;  
> > 
> > Again the somewhat duplicated and "unportable" MemId access.  
> 
> This is in a VAAPI-only function, in spite of the name.
> 
> I think I need to rename some of this stuff to be clearer.  This is really 
> Intel proprietary VAAPI only (and usable as QSV-wrapping-VAAPI), and DXVA2 
> will need separate functions.
> 
> So maybe:
> 
> HAVE_INTEL_OPENCL_VAAPI -> HAVE_INTEL_BEIGNET_OPENCL_VAAPI
> HAVE_INTEL_OPENCL_QSV   -> HAVE_INTEL_MEDIA_OPENCL_VAAPI
> 
> then also HAVE_OPENCL_DXVA2 (which is a standard extension, so not INTEL).

Yeah, I was assuming QSV means QSV on all platforms.

At least HAVE_INTEL_MEDIA_OPENCL_VAAPI sounds like an improvement.

> > Can't comment much. The ifdeffery and complexity of hwcontext_opencl
> > seems a bit worrying, maybe calling for a more structured approach.  
> 
> It could be split across multple files, but there is a lot of shared code so 
> quite a bit of stuff would have to end up in private headers, which didn't 
> seem very nice.  Or did you have something else in mind?

No. If you feel like it'd be an improvement to split it up, I'd
encourage you, otherwise not.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to