| Message ID | 20171008151146.13505-6-sw@jkqxz.net |
|---|---|
| State | New |
| Headers | show |
On 10/8/2017 4:11 PM, Mark Thompson wrote: > Uses vaExportSurfaceHandle() from libva2. > --- > libavutil/hwcontext_vaapi.c | 106 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 104 insertions(+), 2 deletions(-) [...] > + for (i = 0; i < drm_desc->nb_objects; i++) > + close(drm_desc->objects[i].fd); Delightful API... > + surface_id = (VASurfaceID)(uintptr_t)src->data[3]; Can you elaborate a bit on this part? Casting pointers to uintptr_t and storing them is always a red flag to me... C standard issues, etc. > + dst->data[0] = (uint8_t*)drm_desc; This is also a bit wtf-looking... is it cast back at some point? That could be problematic. - Derek
On 08/10/17 17:03, Derek Buitenhuis wrote: > On 10/8/2017 4:11 PM, Mark Thompson wrote: >> Uses vaExportSurfaceHandle() from libva2. >> --- >> libavutil/hwcontext_vaapi.c | 106 +++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 104 insertions(+), 2 deletions(-) > > [...] > >> + for (i = 0; i < drm_desc->nb_objects; i++) >> + close(drm_desc->objects[i].fd); > > Delightful API... > >> + surface_id = (VASurfaceID)(uintptr_t)src->data[3]; > > Can you elaborate a bit on this part? Casting pointers to uintptr_t and storing > them is always a red flag to me... C standard issues, etc. > >> + dst->data[0] = (uint8_t*)drm_desc; > > This is also a bit wtf-looking... is it cast back at some point? That could be > problematic. This is just how hardware surfaces are stored in AVFrames - they have their own API-specific handles in the data[] pointers because that's the only place to put them. See <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/pixfmt.h;h=24889c8e5288cd0e6b6dc59bc4455e87747fa7e8;hb=HEAD#l132> and <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/pixfmt.h;h=24889c8e5288cd0e6b6dc59bc4455e87747fa7e8;hb=HEAD#l337> (and others). Thanks, - Mark
On 10/8/2017 5:11 PM, Mark Thompson wrote: > This is just how hardware surfaces are stored in AVFrames - they have their own API-specific handles in the data[] pointers because that's the only place to put them. > > See > <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/pixfmt.h;h=24889c8e5288cd0e6b6dc59bc4455e87747fa7e8;hb=HEAD#l132> > and > <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/pixfmt.h;h=24889c8e5288cd0e6b6dc59bc4455e87747fa7e8;hb=HEAD#l337> > (and others). > > Thanks, Eugh, well OK. That's arguably not exactly OK by the C standard. Oh well. - Derek
On Sun, 8 Oct 2017 17:13:24 +0100 Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote: > On 10/8/2017 5:11 PM, Mark Thompson wrote: > > This is just how hardware surfaces are stored in AVFrames - they have their own API-specific handles in the data[] pointers because that's the only place to put them. > > > > See > > <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/pixfmt.h;h=24889c8e5288cd0e6b6dc59bc4455e87747fa7e8;hb=HEAD#l132> > > and > > <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/pixfmt.h;h=24889c8e5288cd0e6b6dc59bc4455e87747fa7e8;hb=HEAD#l337> > > (and others). > > > > Thanks, > > Eugh, well OK. That's arguably not exactly OK by the C standard. Oh well. How come? As long as you're strictly casting through intptr_t it should be fine. But yeah, personally I'd prefer having hw surfaces point to some sort of struct instead. Would make more sense with AVBufferRef semantics as well. But a bit too late for that.
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c index b2f2e376d8..3b4ca57d6f 100644 --- a/libavutil/hwcontext_vaapi.c +++ b/libavutil/hwcontext_vaapi.c @@ -884,8 +884,8 @@ fail: return err; } -static int vaapi_map_from(AVHWFramesContext *hwfc, AVFrame *dst, - const AVFrame *src, int flags) +static int vaapi_map_to_memory(AVHWFramesContext *hwfc, AVFrame *dst, + const AVFrame *src, int flags) { int err; @@ -1060,6 +1060,95 @@ static int vaapi_map_from_drm(AVHWFramesContext *src_fc, AVFrame *dst, return 0; } + +static void vaapi_unmap_to_drm(AVHWFramesContext *dst_fc, + HWMapDescriptor *hwmap) +{ + AVDRMFrameDescriptor *drm_desc = hwmap->priv; + int i; + + for (i = 0; i < drm_desc->nb_objects; i++) + close(drm_desc->objects[i].fd); + + av_freep(&drm_desc); +} + +static int vaapi_map_to_drm(AVHWFramesContext *hwfc, AVFrame *dst, + const AVFrame *src, int flags) +{ +#if CONFIG_VAAPI_1 + AVVAAPIDeviceContext *hwctx = hwfc->device_ctx->hwctx; + VASurfaceID surface_id; + VAStatus vas; + VADRMPRIMESurfaceDescriptor va_desc; + AVDRMFrameDescriptor *drm_desc = NULL; + int err, i, j; + + surface_id = (VASurfaceID)(uintptr_t)src->data[3]; + + vas = vaExportSurfaceHandle(hwctx->display, surface_id, + VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME_2, + VA_EXPORT_SURFACE_READ_ONLY | + VA_EXPORT_SURFACE_SEPARATE_LAYERS, + &va_desc); + if (vas != VA_STATUS_SUCCESS) { + av_log(hwfc, AV_LOG_ERROR, "Failed to export surface %#x: " + "%d (%s).\n", surface_id, vas, vaErrorStr(vas)); + return AVERROR(EIO); + } + + drm_desc = av_mallocz(sizeof(*drm_desc)); + if (!drm_desc) { + err = AVERROR(ENOMEM); + goto fail; + } + + // By some bizarre coincidence, these structures are very similar... + drm_desc->nb_objects = va_desc.num_objects; + for (i = 0; i < va_desc.num_objects; i++) { + drm_desc->objects[i].fd = va_desc.objects[i].fd; + drm_desc->objects[i].size = va_desc.objects[i].size; + drm_desc->objects[i].format_modifier = + va_desc.objects[i].drm_format_modifier; + } + drm_desc->nb_layers = va_desc.num_layers; + for (i = 0; i < va_desc.num_layers; i++) { + drm_desc->layers[i].format = va_desc.layers[i].drm_format; + drm_desc->layers[i].nb_planes = va_desc.layers[i].num_planes; + for (j = 0; j < va_desc.layers[i].num_planes; j++) { + drm_desc->layers[i].planes[j].object_index = + va_desc.layers[i].object_index[j]; + drm_desc->layers[i].planes[j].offset = + va_desc.layers[i].offset[j]; + drm_desc->layers[i].planes[j].pitch = + va_desc.layers[i].pitch[j]; + } + } + + err = ff_hwframe_map_create(src->hw_frames_ctx, dst, src, + &vaapi_unmap_to_drm, drm_desc); + if (err < 0) + goto fail; + + dst->width = src->width; + dst->height = src->height; + dst->data[0] = (uint8_t*)drm_desc; + + return 0; + +fail: + for (i = 0; i < va_desc.num_objects; i++) + close(va_desc.objects[i].fd); + av_freep(&drm_desc); + return err; +#else + // Older versions without vaExportSurfaceHandle() are not supported - + // in theory this is possible with a combination of vaDeriveImage() + // and vaAcquireBufferHandle(), but it doesn't carry enough metadata + // to actually use the result in a generic way. + return AVERROR(ENOSYS); +#endif +} #endif static int vaapi_map_to(AVHWFramesContext *hwfc, AVFrame *dst, @@ -1075,6 +1164,19 @@ static int vaapi_map_to(AVHWFramesContext *hwfc, AVFrame *dst, } } +static int vaapi_map_from(AVHWFramesContext *hwfc, AVFrame *dst, + const AVFrame *src, int flags) +{ + switch (dst->format) { +#if CONFIG_LIBDRM + case AV_PIX_FMT_DRM_PRIME: + return vaapi_map_to_drm(hwfc, dst, src, flags); +#endif + default: + return vaapi_map_to_memory(hwfc, dst, src, flags); + } +} + static void vaapi_device_free(AVHWDeviceContext *ctx) { AVVAAPIDeviceContext *hwctx = ctx->hwctx;