Aleksoid1978 / VideoRenderer

Внешний видео-рендерер
GNU General Public License v3.0
1.04k stars 116 forks source link

GetCurrentImage returns image in SAR #9

Closed adipose closed 4 years ago

adipose commented 4 years ago

In contrast to madvr, GetCurrentImage returns image in SAR. As a result code in GetDIB() of mpc-be and mpc-hc returns an image in the wrong aspect ratio.

This is not "wrong" or "right," I suppose. But since mpcvr seems to emulate madvr api somewhat, result is ~identical code with different results.

returns image in SAR:

STDMETHODIMP CMPCVRAllocatorPresenter::GetDIB(BYTE* lpDib, DWORD* size)
{
    HRESULT hr = E_NOTIMPL;
    if (CComQIPtr<IBasicVideo> pBV = m_pMPCVR) {
        hr = pBV->GetCurrentImage((long*)size, (long*)lpDib);
    }
    return hr;
}

returns image in DAR

STDMETHODIMP CmadVRAllocatorPresenter::GetDIB(BYTE* lpDib, DWORD* size)
{
    HRESULT hr = E_NOTIMPL;
    if (CComQIPtr<IBasicVideo> pBV = m_pMVR) {
        hr = pBV->GetCurrentImage((long*)size, (long*)lpDib);
    }
    return hr;
}
Aleksoid1978 commented 4 years ago

Fixed - now GetCurrentImage return image using DAR.

v0lt commented 4 years ago

It is not right. IBasicVideo::GetCurrentImage should return the original image without changing the frame size.

And I did it on purpose. There was no error in the code, but now it is.

adipose commented 4 years ago

@v0lt , thanks for explaining. So madvr is wrong?

Also, if this is known and deliberate, why does mpc-be produce SAR screenshots from MPCVR, but contains patches to use DAR for other video renderers?

In mpc-hc we can adjust after GetCurrentImage. But, since madvr did it this way, I thought perhaps MPCVR should as well. But to be safe, my patch hides the functionality in GetBin rather than overriding GetCurrentImage.

v0lt commented 4 years ago

The renderer is not required to make IBasicVideo::GetCurrentImage frame size adjustments, but some renderers do this.

As far as I remember, VMR-7/9 does not do frame size correction when calling IBasicVideo::GetCurrentImage. Resizing the frame can be done in the player.

I need the ability to get the original frame. I used IBasicVideo::GetCurrentImage for this, but I can’t after the last changes.


A few specifications: IBasicVideo::GetCurrentImage

The GetCurrentImage method retrieves the current image waiting at the renderer.

It used the system VMR-7 and VMR-9.

And in contrast IMFVideoDisplayControl::GetCurrentImage

Gets a copy of the current image being displayed by the video renderer.

It uses the system EVR and produces the displayed frame (the size as it is visible on the display).

adipose commented 4 years ago

This makes sense. If you look at my PR, I did not break GetCurrentImage() (I don't think!).

https://github.com/Aleksoid1978/VideoRenderer/pull/10

I added a GetBin option to pull the data.

Maybe another solution would be an option that could be turned on and off. It could be activated right before calling GetCurrentImage and disabled after.

adipose commented 4 years ago

@Aleksoid1978, so is the DAR fix staying as is? I don't want to get in the middle, but I am unsure after @v0lt has said it is wrong.

Aleksoid1978 commented 4 years ago

Yes - staying as is.