The i965 driver does not free these buffers as required by VAAPI, so we have to do this ourselves if we want to support encoding sessions which run indefinitely.
Since this changes our behaviour to be inconsistent with the specification, also add an extra test on startup to detect non-i965 drivers and log a warning so that the user is aware that there might be problems. --- This is somewhat unfortunate, but I've been sitting on the problem for a while and I don't see any better solution. The i965 driver has been around for too long to change this fundamental behaviour in a new version, and it would take inconveniently long to propagate anyway. (Prompted to do something now by a report in the other tine, see <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-June/195104.html> and associated thread.) libavcodec/vaapi_encode.c | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 45f5e57..50ccbef 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -389,7 +389,31 @@ static int vaapi_encode_issue(AVCodecContext *avctx, av_log(avctx, AV_LOG_ERROR, "Failed to end picture encode issue: " "%d (%s).\n", vas, vaErrorStr(vas)); err = AVERROR(EIO); - goto fail_at_end; + goto fail; + } + + // We free all of the parameter buffers here. + // This is not consistent with what VAAPI states is required; under + // vaDestroyBuffer, it says: "Only call this if the buffer is not + // going to be passed to vaRenderBuffer" [sic; vaRenderPicture]. + // Doing this here is a pragmatic decision to preferentially support + // the Intel i965 back-end driver, which does not and has never + // freed the buffers as required by the specification - if we don't + // free here, memory will leak for every frame submitted when using + // that driver. + // If other drivers are supported in future, this decision may need + // to be revisited - if the driver really has already freed the + // buffer, doing so here is disaster for thread-safety because we + // may free buffers which have been allocated in other threads. + for (i = 0; i < pic->nb_param_buffers; i++) { + vas = vaDestroyBuffer(ctx->hwctx->display, + pic->param_buffers[i]); + if (vas != VA_STATUS_SUCCESS) { + av_log(avctx, AV_LOG_ERROR, "Failed to destroy param " + "buffer %#x: %d (%s).\n", pic->param_buffers[i], + vas, vaErrorStr(vas)); + // And ignore - it will leak. + } } pic->encode_issued = 1; @@ -404,7 +428,6 @@ fail_with_picture: fail: for(i = 0; i < pic->nb_param_buffers; i++) vaDestroyBuffer(ctx->hwctx->display, pic->param_buffers[i]); -fail_at_end: av_freep(&pic->codec_picture_params); av_frame_free(&pic->recon_image); return err; @@ -1034,6 +1057,7 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx, AVHWFramesConstraints *constraints = NULL; enum AVPixelFormat recon_format; VAStatus vas; + const char *vendor_string; int err, i; if (!avctx->hw_frames_ctx) { @@ -1069,6 +1093,16 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx, ctx->device = (AVHWDeviceContext*)ctx->device_ref->data; ctx->hwctx = ctx->device->hwctx; + vendor_string = vaQueryVendorString(ctx->hwctx->display); + if (!vendor_string || !strstr(vendor_string, "i965")) { + av_log(avctx, AV_LOG_WARNING, "Warning: you appear to be " + "using a VAAPI back-end driver which is not the Intel " + "i965 (Quick Sync) driver. This is not tested, may " + "not work, and may have subtle unknown problems due " + "to deviations from documented behaviour required to " + "support the i965 driver.\n"); + } + err = ctx->codec->init(avctx); if (err < 0) goto fail; -- 2.8.1 _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
