Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[d3d11] Implement video processor source rect and tweak filtering. #3970

Merged
merged 2 commits into from
May 2, 2024

Conversation

rbernon
Copy link
Contributor

@rbernon rbernon commented Apr 24, 2024

The use case is the implementation of Media Foundation video processing to convert YUV to RGB D3D buffers.

Media Foundation decoders only support YUV format output, and have some specific behavior where they align their YUV planes on 16 bytes (most common case is 1920x1080 video being padded to 1920x1088, with padding at the end of the Y plane). For some games, we need to be able to convert these aligned YUV buffers to RGB buffers while at the same time cropping or keeping the alignment-induced padding (this depends on the video processor MFT media type attributes).

In short, I'm implementing it like this:

    if (FAILED(hr = IMFMediaType_GetUINT64(processor->input_type, &MF_MT_FRAME_SIZE, &input_desc.frame_size))
            || FAILED(hr = IMFMediaType_GetGUID(processor->input_type, &MF_MT_SUBTYPE, &input_desc.subtype))
            || FAILED(hr = IMFMediaType_GetUINT64(processor->output_type, &MF_MT_FRAME_SIZE, &output_desc.frame_size))
            || FAILED(hr = IMFMediaType_GetGUID(processor->output_type, &MF_MT_SUBTYPE, &output_desc.subtype)))
        return hr;

    /* ... */

    streams.Enable = TRUE;
    streams.OutputIndex = 0;
    streams.InputFrameOrField = 0;
    streams.PastFrames = 0;
    streams.FutureFrames = 0;
    streams.pInputSurface = input_view;

    if (SUCCEEDED(IMFMediaType_GetBlob(processor->input_type, &MF_MT_MINIMUM_DISPLAY_APERTURE, (BYTE *)&aperture, sizeof(aperture), NULL)))
        SetRect(&rect, aperture.OffsetX.value, aperture.OffsetY.value, aperture.OffsetX.value + aperture.Area.cx,
                aperture.OffsetY.value + aperture.Area.cy);
    else
        SetRect(&rect, 0, 0, input_desc.frame_size >> 32, (UINT32)input_desc.frame_size);
    ID3D11VideoContext_VideoProcessorSetStreamSourceRect(video_context, video_processor, 0, TRUE, &rect);

    if (SUCCEEDED(IMFMediaType_GetBlob(processor->output_type, &MF_MT_MINIMUM_DISPLAY_APERTURE, (BYTE *)&aperture, sizeof(aperture), NULL)))
        SetRect(&rect, aperture.OffsetX.value, aperture.OffsetY.value, aperture.OffsetX.value + aperture.Area.cx,
                aperture.OffsetY.value + aperture.Area.cy);
    else
        SetRect(&rect, 0, 0, output_desc.frame_size >> 32, (UINT32)output_desc.frame_size);
    ID3D11VideoContext_VideoProcessorSetStreamDestRect(video_context, video_processor, 0, TRUE, &rect);

    ID3D11VideoContext_VideoProcessorBlt(video_context, video_processor, output_view, 0, 1, &streams);

Currently, as VideoProcessorSetStreamSourceRect does nothing, this ends up scaling the entire input frame into the output rect, which causes the YUV padding to be converted to RGB, causing a green band to be visible (from the Y padding).

Then, when input/output rects are properly scaled onto each other, there's still some color bleed from the padding as linear filtering is used, and a single pixel green line is still visible. Using nearest filtering fixes that. I'm not completely sure what's best here, maybe we can create two (linear + nearest) samplers, and use either one or the other depending on whether the rects dimensions perfectly match.

uboData.coordMatrix[0][0] = 1.0f;
uboData.coordMatrix[1][1] = 1.0f;
uboData.coordMatrix[0][0] = srcRect.width / (float)viewExtent.width;
uboData.coordMatrix[1][1] = srcRect.height / (float)viewExtent.height;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also set the left+top offsets (in [2][0] and [2][1] respectively), or is leaving those out intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly that I didn't need it, but I can add them for completeness.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, would prefer to have that added. PR should be good to once that is done.

@@ -1328,8 +1328,8 @@ namespace dxvk {

void D3D11VideoContext::CreateSampler() {
DxvkSamplerCreateInfo samplerInfo;
samplerInfo.magFilter = VK_FILTER_LINEAR;
samplerInfo.minFilter = VK_FILTER_LINEAR;
samplerInfo.magFilter = VK_FILTER_NEAREST;
Copy link
Owner

@doitsujin doitsujin Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does setting only magFilter to LINEAR work any better than having both LINEAR?

I'd imagine that NEAREST is quite nasty when actually performing any upscaling, but a proper fix that promises to never sample outside the source rect has to be implemented at a shader level and is a bit more complex.

If you have a test case to reproduce the problem with I can look into that once this lands.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not completely sure what you meant, but changing magFilter from LINEAR to NEAREST is what fixes the color bleed and minFilter has no visible effect. I'll update the MR accordingly.

It's related to some larger changes on the Wine side which are not yet merged in Proton, so to reproduce it you will need to build Proton with this wine branch: https://github.com/rbernon/wine/tree/proton-9.0/new-media-source, and then it should happen in Halo Infinite intro videos.

@doitsujin doitsujin merged commit 4333ee8 into doitsujin:master May 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants