[FFmpeg-devel] [PATCH 2/3] libavcodec: v4l2m2m: output AVDRMFrameDescriptor

[Jorge Ramirez-Ortiz]

On 05/09/2018 01:33 AM, Mark Thompson wrote:
>> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
>> index efcb0426e4..9457fadb1e 100644
>> --- a/libavcodec/v4l2_context.c
>> +++ b/libavcodec/v4l2_context.c
>> @@ -393,22 +393,54 @@ static int v4l2_release_buffers(V4L2Context* ctx)
>>       struct v4l2_requestbuffers req = {
>>           .memory = V4L2_MEMORY_MMAP,
>>           .type = ctx->type,
>> -        .count = 0, /* 0 -> unmaps buffers from the driver */
>> +        .count = 0, /* 0 -> unmap all buffers from the driver */
>>       };
>> -    int i, j;
>> +    int ret, i, j;
>>   
>>       for (i = 0; i < ctx->num_buffers; i++) {
>>           V4L2Buffer *buffer = &ctx->buffers[i];
>>   
>>           for (j = 0; j < buffer->num_planes; j++) {
>>               struct V4L2Plane_info *p = &buffer->plane_info[j];
>> +
>> +            if (V4L2_TYPE_IS_OUTPUT(ctx->type)) {
>> +                /* output buffers are not EXPORTED */
>> +                goto unmap;
>> +            }
>> +
>> +            if (ctx_to_m2mctx(ctx)->output_drm) {
>> +                /* use the DRM frame to close */
>> +                if (buffer->drm_frame.objects[j].fd >= 0) {
>> +                    if (close(buffer->drm_frame.objects[j].fd) < 0) {
>> +                        av_log(logger(ctx), AV_LOG_ERROR, "%s close drm fd "
>> +                            "[buffer=%2d, plane=%d, fd=%2d] - %s \n",
>> +                            ctx->name, i, j, buffer->drm_frame.objects[j].fd,
>> +                            av_err2str(AVERROR(errno)));
>> +                    }
>> +                }
>> +            }
>> +unmap:
>>               if (p->mm_addr && p->length)
>>                   if (munmap(p->mm_addr, p->length) < 0)
>> -                    av_log(logger(ctx), AV_LOG_ERROR, "%s unmap plane (%s))\n", ctx->name, av_err2str(AVERROR(errno)));
>> +                    av_log(logger(ctx), AV_LOG_ERROR, "%s unmap plane (%s))\n",
>> +                        ctx->name, av_err2str(AVERROR(errno)));
>>           }
>>       }
>>   
>> -    return ioctl(ctx_to_m2mctx(ctx)->fd, VIDIOC_REQBUFS, &req);
>> +    ret = ioctl(ctx_to_m2mctx(ctx)->fd, VIDIOC_REQBUFS, &req);
>> +    if (ret < 0) {
>> +            av_log(logger(ctx), AV_LOG_ERROR, "release all %s buffers (%s)\n",
>> +                ctx->name, av_err2str(AVERROR(errno)));
>> +
>> +            if (ctx_to_m2mctx(ctx)->output_drm)
>> +                av_log(logger(ctx), AV_LOG_ERROR,
>> +                    "Make sure the DRM client releases all FB/GEM objects before closing the codec (ie):\n"
>> +                    "for all buffers: \n"
>> +                    "  1. drmModeRmFB(..)\n"
>> +                    "  2. drmIoctl(.., DRM_IOCTL_GEM_CLOSE,... )\n");
> You should be able to keep references to all DRM PRIME frames as they leave the codec, and then only call this when all references have disappeared.
>

yes, that is the way it was working for non DRM frames.

If we need to guarantee that exact same behavior, ffmpeg needs to be 
able to remove the fb handles and close the gem objects on each buffer 
being released (so mirror the action we take with just munmap)

This is what I mean:
https://github.com/BayLibre/ffmpeg-drm/blob/master/main.c#L391

I think this would mean that the libavcodec should open the drm device 
instead of the client application doing it and perform the actions above 
in unref.
would that be acceptable?

or perhaps we can just let the libavcodec client be responsible for all 
the drm device actions (which is what this patch does).