On 16/02/16 11:08, wm4 wrote:
> On Mon, 15 Feb 2016 23:24:31 +0000
> Mark Thompson <[email protected]> wrote:
>> ...
>> + MAP(NV12, YUV420, NV12),
>> + MAP(IYUV, YUV420, YUV420P),
>> + MAP(YV12, YUV420, YUV420P), // With U/V planes swapped.
>> + MAP(YV16, YUV422, YUV422P),
>> + MAP(UYVY, YUV422, UYVY422),
>> + //MAP(P010, YUV420_10BPP, P010),
>> + MAP(Y800, YUV400, GRAY8),
>> + MAP(BGRA, RGB32, BGRA),
>> + //MAP(BGRX, RGB32, BGR0),
>> + MAP(RGBA, RGB32, RGBA),
>> + //MAP(RGBX, RGB32, RGB0),
>
> Some of these are commented why? (Sorry if this already came up in the
> ffmpeg patch review.)
These are in ffmpeg but not libav - uncomment on merge.
>> ...
>> +static void vaapi_buffer_free(void *opaque, uint8_t *data)
>> +{
>> + AVHWFramesContext *hwfc = opaque;
>> + AVVAAPIDeviceContext *hwctx = hwfc->device_ctx->hwctx;
>> + VASurfaceID surface_id;
>> + VAStatus vas;
>> +
>> + surface_id = (VASurfaceID)(uintptr_t)data;
>> +
>> + vas = vaDestroySurfaces(hwctx->display, &surface_id, 1);
>> + if (vas != VA_STATUS_SUCCESS) {
>> + av_log(hwfc, AV_LOG_ERROR, "Failed to create surface %#x: "
>> + "%d (%s).\n", surface_id, vas, vaErrorStr(vas));
>> + }
>> +}
>
> If this uses the AVVAAPIDeviceContext, doesn't it need to be unreffed
> here?
Hmm. AVVAAPIDeviceContext is not reference counted (it's the one supplied by
the user when creating the AVHWDeviceContext, which is what we have a reference
to here). Given that it contains the VADisplay, the user has to manage its
lifetime appropriately - even if we made a copy of it, the user could still
close it. If you wanted to guard against that possibility, adding reference
counting to it would be a top-level API change entirely independent of VAAPI.
> Or... I'm not sure if it's sane to access the AVHWFramesContext here,
> because if the AVFrame is supposed to keep it alive, the correctness
> depends on the order AVFrame.buf or AVFrame.hw_frames_ctx is unrefed.
> (IMO it shouldn't matter.)
I think we have to guarantee that the AVHWFramesContext reference (hence
transitively the AVHWDeviceContext) is still present when we deal with buf[0]?
Otherwise, we are just forced to keep another reference somewhere - that ends up
being buf[1] in the absence of anywhere else to put it.
>> ...
>> +static int vaapi_transfer_get_formats(AVHWFramesContext *hwfc,
>> + enum AVHWFrameTransferDirection dir,
>> + enum AVPixelFormat **formats)
>> +{
>> + VAAPIDeviceContext *ctx = hwfc->device_ctx->internal->priv;
>> + enum AVPixelFormat *pix_fmts, preferred_format;
>> + int i, k;
>> +
>> + preferred_format = hwfc->sw_format;
>> +
>> + pix_fmts = av_malloc((ctx->format_count + 1) * sizeof(enum
>> AVPixelFormat));
>> + if (!pix_fmts)
>> + return AVERROR(ENOMEM);
>> +
>> + pix_fmts[0] = preferred_format;
>> + k = 1;
>> + for (i = 0; i < ctx->format_count; i++) {
>> + if (ctx->format_list[i].pix_fmt == preferred_format)
>> + continue;
>> + av_assert0(k < ctx->format_count);
>> + pix_fmts[k++] = ctx->format_list[i].pix_fmt;
>
> So, they're returned in random order.
Preferred format first, then all other supported formats in random order. I
don't think there is anything else useful to do here - only the preferred format
can possibly work with direct mapping, the others will all have a conversion
step and we don't have any way to distinguish between them.
>> + }
>> + av_assert0(k == ctx->format_count);
>> + pix_fmts[k] = AV_PIX_FMT_NONE;
>> +
>> + *formats = pix_fmts;
>> + return 0;
>> +}
>> ...
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
