Generally looks good, though I'm not very familiar with this hwaccel.
Are you planning to deprecate the old-style hwaccels?
Some smaller comments follow:
Quoting wm4 (2017-05-04 08:44:08)
> +#if CONFIG_D3D11VA
> +
> +static int d3d11va_get_decoder_configuration(AVCodecContext *avctx,
> + ID3D11VideoDevice *video_device,
> + const D3D11_VIDEO_DECODER_DESC
> *desc,
> + D3D11_VIDEO_DECODER_CONFIG
> *config)
> +{
> + FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx);
> + unsigned cfg_count = 0;
> + D3D11_VIDEO_DECODER_CONFIG *cfg_list = NULL;
> + HRESULT hr;
> + int i, ret;
> +
> + hr = ID3D11VideoDevice_GetVideoDecoderConfigCount(video_device, desc,
> &cfg_count);
> + if (FAILED(hr)) {
> + av_log(avctx, AV_LOG_ERROR, "Unable to retrieve decoder
> configurations\n");
> + return AVERROR(EINVAL);
> + }
> +
> + cfg_list = av_malloc(cfg_count * sizeof(D3D11_VIDEO_DECODER_CONFIG));
nit: av_malloc_array()
> + if (cfg_list == NULL)
> + return AVERROR(ENOMEM);
> + for (i = 0; i < cfg_count; i++) {
> + hr = ID3D11VideoDevice_GetVideoDecoderConfig(video_device, desc, i,
> &cfg_list[i]);
> + if (FAILED(hr)) {
> + av_log(avctx, AV_LOG_ERROR, "Unable to retrieve decoder
> configurations. (hr=0x%lX)\n", hr);
> + free(cfg_list);
av_free(p)
> + return AVERROR(EINVAL);
> + }
> + }
> +
> + ret = dxva_get_decoder_configuration(avctx, cfg_list, cfg_count);
> + if (ret >= 0)
> + *config = cfg_list[ret];
> + av_free(cfg_list);
> + return ret;
> +}
> +
> +static int d3d11va_create_decoder(AVCodecContext *avctx)
> +{
> + FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx);
> + GUID *guid_list;
> + unsigned guid_count, i;
> + GUID decoder_guid;
> + DXGI_FORMAT surface_format = avctx->sw_pix_fmt == AV_PIX_FMT_YUV420P10 ?
> + DXGI_FORMAT_P010 : DXGI_FORMAT_NV12;
> + D3D11_VIDEO_DECODER_DESC desc = { 0 };
> + D3D11_VIDEO_DECODER_CONFIG config;
> + AVHWFramesContext *frames_ctx = (AVHWFramesContext
> *)avctx->hw_frames_ctx->data;
> + AVD3D11VADeviceContext *device_hwctx = frames_ctx->device_ctx->hwctx;
> + AVD3D11VAFramesContext *frames_hwctx = frames_ctx->hwctx;
> + D3D11_TEXTURE2D_DESC texdesc;
> + HRESULT hr;
> + int ret;
> +
> + if (!frames_hwctx->texture) {
> + av_log(avctx, AV_LOG_ERROR, "AVD3D11VAFramesContext.texture not
> set.\n");
> + return AVERROR(EINVAL);
> + }
> + ID3D11Texture2D_GetDesc(frames_hwctx->texture, &texdesc);
> +
> + guid_count =
> ID3D11VideoDevice_GetVideoDecoderProfileCount(device_hwctx->video_device);
> + guid_list = av_malloc(sizeof(*guid_list) * guid_count);
I'd expect that you need to check that guid_count is positive.
nit: av_malloc_array()
Also, weird inconsistent formatting in the rest of this block.
> +
> +int ff_dxva2_decode_uninit(AVCodecContext *avctx)
> +{
> + FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx);
> + int i;
> +
> + av_buffer_unref(&sctx->decoder_ref);
> +
> +#if CONFIG_D3D11VA
> + for (i = 0; i < sctx->nb_d3d11_views; i++) {
> + if (sctx->d3d11_views[i])
> + ID3D11VideoDecoderOutputView_Release(sctx->d3d11_views[i]);
> + }
> + av_free(sctx->d3d11_views);
nit: av_freep() looks safer here
(I tend to just use it everywhere and never use av_free since there's no
reason not to)
> +#endif
> +
> +#if CONFIG_DXVA2
> + if (sctx->dxva2_service)
> + IDirectXVideoDecoderService_Release(sctx->dxva2_service);
> +#endif
> +
> + av_buffer_unref(&avctx->hw_frames_ctx);
Why? This should already be done at the higher level, no?
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
