On 09/05/17 06:07, wm4 wrote:
> On Mon, 8 May 2017 22:59:11 +0100
> Mark Thompson <[email protected]> wrote:
>> On 04/05/17 07:44, wm4 wrote:
>>> To be used with the new d3d11 hwaccel decode API.
>>>
>>> With the new hwaccel API, we don't want surfaces to depend on the
>>> decoder (other than the required dimension and format). The old D3D11VA
>>> pixfmt uses ID3D11VideoDecoderOutputView pointers, which include the
>>> decoder configuration, and thus is incompatible with the new hwaccel
>>> API. This patch introduces AV_PIX_FMT_D3D11, which uses ID3D11Texture2D
>>> and an index. It's simpler and compatible with the new hwaccel API.
>>>
>>> The introduced hwcontext supports only the new pixfmt.
>>>
>>> Significantly based on work by Steve Lhomme <[email protected]>, but with
>>> heavy changes/rewrites.
>
>
>>> ---
>>> Somewhat sketchy: if initial_pool_size is set, the pool is assumed to
>>> be static.
>>> ---
>
> Any comments on that?
It's how all of the other implementations with fixed pools work, so fine?
>>> +static int d3d11va_transfer_data(AVHWFramesContext *ctx, AVFrame *dst,
>>> + const AVFrame *src)
>>> +{
>>> + AVD3D11VADeviceContext *device_hwctx = ctx->device_ctx->hwctx;
>>> + D3D11VAFramesContext *s = ctx->internal->priv;
>>> + int download = !!src->hw_frames_ctx;
>>
>> This isn't correct - src could have hw_frames_ctx set as a mapped frame
>> while we are really uploading to dst. I don't think you can write this with
>> only one function.
>>
>> (Not sure there is any case where you would ever want to do that, though.)
>
> OK I guess.
>
>>> + const AVFrame *frame = download ? src : dst;
>>> + // (The interface types are compatible.)
>>> + ID3D11Resource *texture = (ID3D11Resource *)(ID3D11Texture2D
>>> *)frame->data[0];
>>> + int index = (intptr_t)frame->data[1];
>>> + ID3D11Resource *staging = (ID3D11Resource *)s->staging_texture;
>>> + int w = FFMIN(dst->width, src->width);
>>> + int h = FFMIN(dst->height, src->height);
>>> + uint8_t *map_data[4];
>>> + int map_linesize[4];
>>> + D3D11_TEXTURE2D_DESC desc;
>>> + D3D11_MAPPED_SUBRESOURCE map;
>>> + HRESULT hr;
>>> +
>>> + device_hwctx->lock(device_hwctx->lock_ctx);
>>> +
>>> + ID3D11Texture2D_GetDesc(s->staging_texture, &desc);
>>> +
>>> + if (download) {
>>> +
>>> ID3D11DeviceContext_CopySubresourceRegion(device_hwctx->device_context,
>>> + staging, 0, 0, 0, 0,
>>> + texture, index, NULL);
>>> +
>>> + hr = ID3D11DeviceContext_Map(device_hwctx->device_context,
>>> + staging, 0, D3D11_MAP_READ, 0, &map);
>>> + if (FAILED(hr))
>>> + goto map_failed;
>>> +
>>> + fill_texture_ptrs(map_data, map_linesize, ctx, &desc, &map);
>>> +
>>> + av_image_copy(dst->data, dst->linesize, map_data, map_linesize,
>>> + ctx->sw_format, w, h);
>>
>> Is the staging texture somehow magic such that it isn't going to be uncached
>> memory or anything tricky like that?
>
> Yes, I'm fairly sure it simply lives in CPU memory, and
> CopySubresourceRegion() will access the actual video memory.
>
> This is also why d3d11 with copy back is slower than dxva2, apparently.
Well, at least it's probably faster on Braswell :P
>>> +
>>> + ID3D11DeviceContext_Unmap(device_hwctx->device_context, staging,
>>> 0);
>>> + } else {
>>> + hr = ID3D11DeviceContext_Map(device_hwctx->device_context,
>>> + staging, 0, D3D11_MAP_WRITE, 0, &map);
>>> + if (FAILED(hr))
>>> + goto map_failed;
>>> +
>>> + fill_texture_ptrs(map_data, map_linesize, ctx, &desc, &map);
>>> +
>>> + av_image_copy(map_data, map_linesize, src->data, src->linesize,
>>> + ctx->sw_format, w, h);
>>> +
>>> + ID3D11DeviceContext_Unmap(device_hwctx->device_context, staging,
>>> 0);
>>> +
>>> +
>>> ID3D11DeviceContext_CopySubresourceRegion(device_hwctx->device_context,
>>> + texture, index, 0, 0, 0,
>>> + staging, 0, NULL);
>>> + }
>>> +
>>> + device_hwctx->unlock(device_hwctx->lock_ctx);
>>> + return 0;
>>> +
>>> +map_failed:
>>> + av_log(ctx, AV_LOG_ERROR, "Unable to lock D3D11VA surface (%lx)\n",
>>> (long)hr);
>>> + device_hwctx->unlock(device_hwctx->lock_ctx);
>>> + return AVERROR_UNKNOWN;
>>> +}
>>
>> Seems like it would be nicer to implement map and use it rather than putting
>> it multiple times inside this function.
>
> Maybe. I'm unsure about the mapping semantics. Will look at it again.
>
>>> + AV_PIX_FMT_D3D11, ///< HW decoding through Direct3D11 via new API,
>>> Picture.data[0] contains a ID3D11Texture2D pointer, and data[1] contains
>>> the texture array index of the frame as intptr_t if the ID3D11Texture2D is
>>> an array texture (or 0 if it's a normal texture)
>>
>> The old format was specifically for decoding, but the new one is more
>> general than that.
>
> OK. Well, someone new to the API who's trying to figure out which
> format to use is going to be confused, so mentioning the new decode API
> might help.
Yeah, fair. Mention decoding but not as the only thing, then.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
