[FFmpeg-devel,6/6] hwcontext_vaapi: Add support for mapping to DRM objects

Submitted by Mark Thompson on Oct. 8, 2017, 3:11 p.m.

Details

Message ID 20171008151146.13505-6-sw@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson Oct. 8, 2017, 3:11 p.m.
Uses vaExportSurfaceHandle() from libva2.
---
 libavutil/hwcontext_vaapi.c | 106 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 2 deletions(-)

Comments

Derek Buitenhuis Oct. 8, 2017, 4:03 p.m.
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
Mark Thompson Oct. 8, 2017, 4:11 p.m.
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
Derek Buitenhuis Oct. 8, 2017, 4:13 p.m.
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
wm4 Oct. 9, 2017, 11:25 a.m.
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.

Patch hide | download patch | download mbox

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;