Re: [PATCH 3/4] quartz: Try all surfaces when calling GetSurface
On Wed, Jul 20, 2016 at 02:37:11PM +1000, Jan Schmidt wrote:
If the surface we want is refused, cycle through all possible ones. Some applications (Silver at least) request 2 surfaces, but then refuse to return anything other than the first one.
I'm not necessarily opposed to this, but it does seem unlikely that the application would request two surfaces be used, but only provide one. I noticed that we set num_surfaces to 2 in VMR9_maybe_init. Is this function calling IVMRSurfaceAllocatorEx9::InitializeDevice on the application-provided allocator, or on our default implementation? This seems like where the application should tell us it wants only one surface. If it is calling the application's allocator, then maybe they don't handle VMR9AllocationInfo::MinBuffers > 1 very well. Why do we set MinBuffers to 2 here, I wonder. Aric? Andrew
Signed-off-by: Jan Schmidt <jan(a)centricular.com> --- dlls/quartz/vmr9.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index d590a71..8455678 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -292,6 +292,7 @@ static HRESULT WINAPI VMR9_DoRenderSample(BaseRenderer *iface, IMediaSample * pS REFERENCE_TIME tStart, tStop; VMR9PresentationInfo info; HRESULT hr; + int i;
TRACE("%p %p\n", iface, pSample);
@@ -338,10 +339,19 @@ static HRESULT WINAPI VMR9_DoRenderSample(BaseRenderer *iface, IMediaSample * pS info.szAspectRatio.cx = This->bmiheader.biWidth; info.szAspectRatio.cy = This->bmiheader.biHeight;
- hr = IVMRSurfaceAllocatorEx9_GetSurface(This->allocator, This->cookie, (++This->cur_surface)%This->num_surfaces, 0, &info.lpSurf); + /* Sometimes apps refuse to return the surface we request, + * so try each and see if any are available if the first fails */ + for (i = 0; i < This->num_surfaces; i++) { + hr = IVMRSurfaceAllocatorEx9_GetSurface(This->allocator, This->cookie, + (++This->cur_surface) % This->num_surfaces, 0, &info.lpSurf); + if (SUCCEEDED (hr)) + break; + }
- if (FAILED(hr)) + if (FAILED(hr)) { + ERR("Failed to get render surface (%x)\n", hr); return hr; + }
VMR9_SendSampleData(This, &info, pbSrcStream, cbSrcStream); IDirect3DSurface9_Release(info.lpSurf); -- 2.7.4
On 21/07/16 01:14, Andrew Eikum wrote:
On Wed, Jul 20, 2016 at 02:37:11PM +1000, Jan Schmidt wrote:
If the surface we want is refused, cycle through all possible ones. Some applications (Silver at least) request 2 surfaces, but then refuse to return anything other than the first one.
I'm not necessarily opposed to this, but it does seem unlikely that the application would request two surfaces be used, but only provide one.
I noticed that we set num_surfaces to 2 in VMR9_maybe_init. Is this function calling IVMRSurfaceAllocatorEx9::InitializeDevice on the application-provided allocator, or on our default implementation? This seems like where the application should tell us it wants only one surface.
It calls the allocator the app provide earlier in VMR9SurfaceAllocatorNotify_AdviseSurfaceAllocator which is then passing it to VMR9SurfaceAllocatorNotify_AllocateSurfaceHelper to do the actual allocation. I think it's just passing the allocation info intact - including the MinBuffers and default buffers=2 value (verified by changing those). I think the app then only stores the first surface though.
If it is calling the application's allocator, then maybe they don't handle VMR9AllocationInfo::MinBuffers > 1 very well. Why do we set MinBuffers to 2 here, I wonder. Aric?
Since we're asking design questions: It requests surfaces with VMR9AllocFlag_TextureSurface, but D3D doesn't seem to support 24-bit RGB textures. I only have 1 test case for the VMR9 renderer (Silver), and for that the filter graph plugs the GStreamer YUV2RGB component, which outputs 24-bit RGB. I'm using a local patch to output RGBA instead to make things work. My plan is to extend the TransformFilter base class and make the YUV2RGB filter negotiate with the VMR9 filter and do 32-bit anyway, but it seems the VMR9 filter should not advertise 24-bit support at all? J.
Andrew
Signed-off-by: Jan Schmidt <jan(a)centricular.com> --- dlls/quartz/vmr9.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index d590a71..8455678 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -292,6 +292,7 @@ static HRESULT WINAPI VMR9_DoRenderSample(BaseRenderer *iface, IMediaSample * pS REFERENCE_TIME tStart, tStop; VMR9PresentationInfo info; HRESULT hr; + int i;
TRACE("%p %p\n", iface, pSample);
@@ -338,10 +339,19 @@ static HRESULT WINAPI VMR9_DoRenderSample(BaseRenderer *iface, IMediaSample * pS info.szAspectRatio.cx = This->bmiheader.biWidth; info.szAspectRatio.cy = This->bmiheader.biHeight;
- hr = IVMRSurfaceAllocatorEx9_GetSurface(This->allocator, This->cookie, (++This->cur_surface)%This->num_surfaces, 0, &info.lpSurf); + /* Sometimes apps refuse to return the surface we request, + * so try each and see if any are available if the first fails */ + for (i = 0; i < This->num_surfaces; i++) { + hr = IVMRSurfaceAllocatorEx9_GetSurface(This->allocator, This->cookie, + (++This->cur_surface) % This->num_surfaces, 0, &info.lpSurf); + if (SUCCEEDED (hr)) + break; + }
- if (FAILED(hr)) + if (FAILED(hr)) { + ERR("Failed to get render surface (%x)\n", hr); return hr; + }
VMR9_SendSampleData(This, &info, pbSrcStream, cbSrcStream); IDirect3DSurface9_Release(info.lpSurf); -- 2.7.4
-- Jan Schmidt, Centricular Ltd - http://centricular.com/
participants (2)
-
Andrew Eikum -
Jan Schmidt