August 2017 - Wine-Devel - list.winehq.org

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer

by Fabian Maurer

> >> Shouldn't it use strmbase? > > > > You mean the methods that for example quartz/nullrenderer.c uses in > > NullRenderer_Vtbl? > > I'm not sure, that's why I just stubbed them. > > I mean BaseFilter from strmbase. Ah you mean I shouldn't implement this interface by myself, but just return a BaseFilter object? I'm not sure how I would test against that to make sure it actually is a BaseFilter and not something different. > >> This needs at test for aggregation support. > > > > Sorry I don't know what you mean. Care to elaborate? > > I mean a test to show if it's supported or not. Currently you ignore > outer interface argument. Take a look at qcap for example. I see, I never heard of COM aggregation before, so I first need to read into it. > I mean if there's no reason, why should it be in a header. > Using just "%p" is enough for pointers, you don't need extra prefix. > I personally don't think d3dxof is a good place to look for examples. > But anyway copied parts should get some cleanup and not be taken for > granted. XF prefix for example could possibly be an acronym for > directXFile, and that does not apply to anything but d3dxof. Makes sense, I will rework that. Regards, Fabian Maurer

Aug. 18, 2017

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer

by Nikolay Sivov

On 18.08.2017 20:01, Fabian Maurer wrote: > Hi, thanks for your feedback. > >> Shouldn't it use strmbase? > > You mean the methods that for example quartz/nullrenderer.c uses in > NullRenderer_Vtbl? > I'm not sure, that's why I just stubbed them. I mean BaseFilter from strmbase. > >> This needs at test for aggregation support. > > Sorry I don't know what you mean. Care to elaborate? I mean a test to show if it's supported or not. Currently you ignore outer interface argument. Take a look at qcap for example. > >>> +typedef struct { >>> + IBaseFilter IBaseFilter_iface; >>> + LONG ref; >>> +} EnhancedVideoRendererImpl; >> >> Does this need to be exposed? > > You mean I shouldn't need to put that in a header? I just did it like in > d3dxof, and it's the same in uiribbon. But yes, I could change that. I mean if there's no reason, why should it be in a header. > >>> + TRACE("(0x%p, %d, %p)\n", instance, reason, reserved); >>> + >> Format looks wrong. > > Already used that in my uiribbon patch and it was fine though. What should I > change about it? Using just "%p" is enough for pointers, you don't need extra prefix. > >> What does XF prefix stand for? Also please use consistent type names and >> formatting. >> >> I personally don't think this export needs a documentation header. >> >> I think it's better to let QueryInterface handle this. > > I just copied that code from d3dxof, I figured it was ok since it was accepted > into my uiribbon patch. Should I change it? I personally don't think d3dxof is a good place to look for examples. But anyway copied parts should get some cleanup and not be taken for granted. XF prefix for example could possibly be an acronym for directXFile, and that does not apply to anything but d3dxof. > >>> + for (i=0; i < sizeof(object_creation)/sizeof(object_creation[0]); >>> i++) >>> + { >>> + if (IsEqualGUID(object_creation[i].clsid, rclsid)) >>> + break; >>> + } >>> + >>> + if (i == sizeof(object_creation)/sizeof(object_creation[0])) >>> + { >>> + FIXME("%s: no class found.\n", debugstr_guid(rclsid)); >>> + return CLASS_E_CLASSNOTAVAILABLE; >>> + } >> >> Does it support any other CLSIDs? This seems like too much. Static >> factory instance will work too I suppose. > > According to my windows7 registry, yes, there are other CLSIDs in that dll, > like "MF Video Mixer" and "Tearless Window Presenter" for example. Ok, then this structure is fine I guess. Thanks for checking. > Not sure what exactly static factory instance means. I mean a static, non-heap allocated, instance. But that's minor. > > Regards, > Fabian Maurer > >

Aug. 18, 2017

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer

by Fabian Maurer

Hi, thanks for your feedback. > Shouldn't it use strmbase? You mean the methods that for example quartz/nullrenderer.c uses in NullRenderer_Vtbl? I'm not sure, that's why I just stubbed them. > This needs at test for aggregation support. Sorry I don't know what you mean. Care to elaborate? > > +typedef struct { > > + IBaseFilter IBaseFilter_iface; > > + LONG ref; > > +} EnhancedVideoRendererImpl; > > Does this need to be exposed? You mean I shouldn't need to put that in a header? I just did it like in d3dxof, and it's the same in uiribbon. But yes, I could change that. > > + TRACE("(0x%p, %d, %p)\n", instance, reason, reserved); > > + > Format looks wrong. Already used that in my uiribbon patch and it was fine though. What should I change about it? > It still doesn't do anything. Yes, I'll re-add that. > What does XF prefix stand for? Also please use consistent type names and > formatting. > > I personally don't think this export needs a documentation header. > > I think it's better to let QueryInterface handle this. I just copied that code from d3dxof, I figured it was ok since it was accepted into my uiribbon patch. Should I change it? > > + for (i=0; i < sizeof(object_creation)/sizeof(object_creation[0]); > > i++) > > + { > > + if (IsEqualGUID(object_creation[i].clsid, rclsid)) > > + break; > > + } > > + > > + if (i == sizeof(object_creation)/sizeof(object_creation[0])) > > + { > > + FIXME("%s: no class found.\n", debugstr_guid(rclsid)); > > + return CLASS_E_CLASSNOTAVAILABLE; > > + } > > Does it support any other CLSIDs? This seems like too much. Static > factory instance will work too I suppose. According to my windows7 registry, yes, there are other CLSIDs in that dll, like "MF Video Mixer" and "Tearless Window Presenter" for example. Not sure what exactly static factory instance means. Regards, Fabian Maurer

Aug. 18, 2017

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer

by Fabian Maurer

Hi Alistair, > Your missing some IID checks here. You mean for example the DirectShow interfaces like "IAMCertifiedOutputProtection" "IMediaEventSink" and so on? I skipped them for now, because they are not needed to fix that particular bug. Do I have to add all of them at once? Regards, Fabian Maurer

Aug. 18, 2017

Re: [1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.

by Dmitry Timoshkov

Sebastian Lackner <sebastian(a)fds-team.de> wrote: > On 18.08.2017 13:41, Dmitry Timoshkov wrote: > > Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> > > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > > --- > > dlls/user32/tests/dialog.c | 6 ++++++ > > dlls/user32/tests/resource.rc | 6 ++++++ > > 2 files changed, 12 insertions(+) > > > > There is no need to resubmit, the patches are exactly identical to > what Andrew submitted. Also, why the hell is there my sign-off? It is > apparently not fine for you when Andrew wants to take responsiblity > for your patches, but you expect me to take responsibility for it, > without confirming that I'm fine with it first? For other people I > would now start to explain that Wine Staging also accepts workarounds > and imperfect patches, and that the addition to Wine Staging does not > imply my sign-off, but I'm pretty sure you are well aware of it... I'm sorry about adding your sign-off, probably I should have longer waiting for your reply before submitting, and apparently I was too optimistic about implicit sign-off of the wine-staging maintainer when adding the patches... -- Dmitry.

Aug. 18, 2017

Re: [1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=32738 Your paranoid android. === w7u (32 bit msg) === msg.c:5084: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088 msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 11: the msg 0x001c was expected, but got msg 0x0047 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 12: the msg 0x0086 was expected, but got msg 0x001c instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 13: the msg 0x0006 was expected, but got msg 0x0086 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 18: the msg 0x0009 was expected, but got msg 0x0006 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 19: the msg 0x0007 was expected, but got msg 0x0009 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 20: the msg 0x0047 was expected, but got msg 0x0007 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 21: the msg 0x0003 was expected, but got msg 0x0047 instead msg.c:5207: Test failed: SetWindowPos:show_popup_first_show_window: 22: the msg sequence is not complete: expected 0000 - actual 0003 === wvistau64 (32 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === wvistau64 (64 bit listbox) === listbox.c:706: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY, *) filled with 7 entries, expected > 7 listbox.c:941: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE, *) failed err 18 === w864 (32 bit broadcast) === The task timed out === w8 (32 bit sysparams) === sysparams.c:2519: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2530: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w1064 (32 bit sysparams) === sysparams.c:206: Test failed: Unexpected WM_DISPLAYCHANGE message === w864 (64 bit sysparams) === The task timed out

Aug. 18, 2017

Re: [1/2] user32/tests: Test DialogBoxParam using a dialog template with invalid control class.

by Sebastian Lackner

On 18.08.2017 13:41, Dmitry Timoshkov wrote: > Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > dlls/user32/tests/dialog.c | 6 ++++++ > dlls/user32/tests/resource.rc | 6 ++++++ > 2 files changed, 12 insertions(+) > There is no need to resubmit, the patches are exactly identical to what Andrew submitted. Also, why the hell is there my sign-off? It is apparently not fine for you when Andrew wants to take responsiblity for your patches, but you expect me to take responsibility for it, without confirming that I'm fine with it first? For other people I would now start to explain that Wine Staging also accepts workarounds and imperfect patches, and that the addition to Wine Staging does not imply my sign-off, but I'm pretty sure you are well aware of it...

Aug. 18, 2017

Re: JACK driver for WINE

by Andrew Eikum

On Thu, Aug 17, 2017 at 01:14:00PM -0600, Jeremy Banks wrote: > I am wanting to output the audio from various applications run in WINE to > the JACK server on my computer. I would prefer that WINE do this natively > instead of routing across various wrapper setups with ALSA or PulseAudio. I > have looked on the WINE wiki page for sound[1] and see that there was a > JACK driver at one point, but it was removed when the driver architecture > was changed. > Hey Jeremy, thanks for your interest! Like Austin said, I do most of the audio stuff in Wine these days, so you'll probably be working mostly with me. First, I'd request that you try out the alternatives to implementing a driver. I don't know what your requirements are, but there are existing solutions for running PulseAudio on top of JACK. I'm not super familiar with JACK, but I think you'll want to follow the "Redirecting PulseAudio to JACK" section here[1], then make sure Wine is using the winepulse driver. If that doesn't meet your requirements, you could also try redirecting winealsa to JACK, see[2,3]. This may have lower latency than running PA. The reason we resist adding new drivers is it adds a lot of work for the maintainers (i.e., me). I test each significant change to any part of our audio code on all four of our existing drivers (ALSA, OSS, PulseAudio, and macOS's CoreAudio), both by running the Wine unit tests and by testing all of the many audio applications I have that use the relevant APIs. On top of the testing itself, that means I have to understand how to run and maintain all four of those audio systems on my hardware. Further, any bug fixed or new feature added to one driver has to be understood and replicated in all other drivers, if applicable. So you can see why I'd request you try existing solutions before jumping straight to a new audio driver. If you can convince me that existing solutions don't work for your requirements, then we can talk about adding a new driver to Wine. I'd at least like to be convinced that you've already tried alternatives and understand why they are unacceptable for your usecase. > I am interested in trying to implement a driver to get WINE talking to JACK > natively. The wiki says to ask on the mailing list before starting such a > project, and so here I am. I have downloaded a local copy of the source repo > and have started browsing through its contents. Being unfamiliar with the > WINE codebase, which portions of the source should I start studying first? I > am interested in studying the drivers for ALSA, OSS, and Pulse for > comparison; do those live in a particular place in the source tree? I'm > presently looking on the Wiki for other helpful information, but is there > some particular documentation I should be studying for this endeavor? > Once you get over that hurdle, you'll need to implement a winejack.drv similar to winealsa, winepulse, winecoreaudio, or wineoss. You'll want to copy whichever is closest to JACK's API and then modify it to use JACK. winealsa and wineoss operate in push mode (we maintain our own timers to keep the audio buffer full). winepulse and winecoreaudio are in pull mode (the audio system asks us to provide it with data). I don't recall which type of API JACK is. At a very high level, you'll need to implement all of the functions in the .spec file. You can look into the existing drivers to understand what those APIs do. The audio drivers export a few COM interfaces, which are used directly by applications or by dsound, winmm, and some other modules. The really important interfaces are IAudioClient, IAudioRenderClient, IAudioCaptureClient, and IAudioClock, and there are a few lesser-used interfaces besides. Once you've implemented those interfaces, they'll need to pass the tests mentioned on the wiki page. [1] https://github.com/jackaudio/jackaudio.github.com/wiki/WalkThrough_User_Pul… [2] https://wiki.winehq.org/Sound#What_about_JACK.3F [3] http://jackaudio.org/faq/routing_alsa.html Andrew

Aug. 18, 2017

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer

by Alistair Leslie-Hughes

Hi Fabian, On 18/08/17 11:25, Fabian Maurer wrote: > + if (IsEqualGUID(riid, &IID_IUnknown) > + || IsEqualGUID(riid, &IID_IBaseFilter)) > + { Your missing some IID checks here. Alistair.

Aug. 18, 2017

Re: [PATCH] evr: Add stubs for EnhancedVideoRenderer

by Nikolay Sivov

On 18.08.2017 4:25, Fabian Maurer wrote: > +static const IBaseFilterVtbl IBaseFilter_Vtbl = > +{ > + /*** IUnknown methods ***/ > + EnhancedVideoRendererImpl_QueryInterface, > + EnhancedVideoRendererImpl_AddRef, > + EnhancedVideoRendererImpl_Release, > + /*** IPersist methods ***/ > + EnhancedVideoRendererImpl_GetClassID, > + /*** IMediaFilter methods ***/ > + EnhancedVideoRendererImpl_Stop, > + EnhancedVideoRendererImpl_Pause, > + EnhancedVideoRendererImpl_Run, > + EnhancedVideoRendererImpl_GetState, > + EnhancedVideoRendererImpl_SetSyncSource, > + EnhancedVideoRendererImpl_GetSyncSource, > + /*** IBaseFilter methods ***/ > + EnhancedVideoRendererImpl_EnumPins, > + EnhancedVideoRendererImpl_FindPin, > + EnhancedVideoRendererImpl_QueryFilterInfo, > + EnhancedVideoRendererImpl_JoinFilterGraph, > + EnhancedVideoRendererImpl_QueryVendorInfo > +}; Shouldn't it use strmbase? > +HRESULT EnhancedVideoRendererImpl_Create(IUnknown *pUnkOuter, void **ppObj) > +{ > + EnhancedVideoRendererImpl *object; > + > + TRACE("(%p,%p)\n", pUnkOuter, ppObj); > + > + object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(EnhancedVideoRendererImpl)); > + if (!object) > + return E_OUTOFMEMORY; > + > + object->IBaseFilter_iface.lpVtbl = &IBaseFilter_Vtbl; > + object->ref = 1; > + > + *ppObj = &object->IBaseFilter_iface; > + > + return S_OK; > +} This needs at test for aggregation support. > +typedef struct { > + IBaseFilter IBaseFilter_iface; > + LONG ref; > +} EnhancedVideoRendererImpl; Does this need to be exposed? > + TRACE("(0x%p, %d, %p)\n", instance, reason, reserved); > + Format looks wrong. > - case DLL_WINE_PREATTACH: > - return FALSE; /* prefer native version */ It still doesn't do anything. > +static HRESULT WINAPI XFCF_QueryInterface(LPCLASSFACTORY iface,REFIID riid, void **ppobj) > +{ What does XF prefix stand for? Also please use consistent type names and formatting. > +/******************************************************************************* > + * Retrieves class object from a DLL object > + * > + * NOTES > + * Docs say returns STDAPI > + * > + * PARAMS > + * rclsid [I] CLSID for the class object > + * riid [I] Reference to identifier of interface for class object > + * ppv [O] Address of variable to receive interface pointer for riid > + * > + * RETURNS > + * Success: S_OK > + * Failure: CLASS_E_CLASSNOTAVAILABLE, E_OUTOFMEMORY, E_INVALIDARG, > + * E_UNEXPECTED > + */ > +HRESULT WINAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, void **ppv) I personally don't think this export needs a documentation header. > + if ( !IsEqualGUID( &IID_IClassFactory, riid ) > + && ! IsEqualGUID( &IID_IUnknown, riid) ) > + return E_NOINTERFACE; I think it's better to let QueryInterface handle this. > + for (i=0; i < sizeof(object_creation)/sizeof(object_creation[0]); i++) > + { > + if (IsEqualGUID(object_creation[i].clsid, rclsid)) > + break; > + } > + > + if (i == sizeof(object_creation)/sizeof(object_creation[0])) > + { > + FIXME("%s: no class found.\n", debugstr_guid(rclsid)); > + return CLASS_E_CLASSNOTAVAILABLE; > + } Does it support any other CLSIDs? This seems like too much. Static factory instance will work too I suppose.

Aug. 18, 2017