Re: [patch 1/1] dlls/windowscodecs/pngformat.c: implemented 'PngDecoder_Block_GetCount'
On 10/5/2012 06:43, max+...@mtew.isa-geek.net wrote:
From: Max TenEyck Woodbury max+...@mtew.isa-geek.net
---
dlls/windowscodecs/pngformat.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/windowscodecs/pngformat.c b/dlls/windowscodecs/pngformat.c
index 686f9c6..e8e7cbe 100644
--- a/dlls/windowscodecs/pngformat.c
+++ b/dlls/windowscodecs/pngformat.c
@@ -899,8 +899,10 @@ static HRESULT WINAPI
PngDecoder_Block_GetContainerFormat(IWICMetadataBlockReade
static HRESULT WINAPI PngDecoder_Block_GetCount(IWICMetadataBlockReader
*iface,
UINT *pcCount)
{
-FIXME(%p,%p: stub\n, iface, pcCount);
-return E_NOTIMPL;
+PngDecoder *This = impl_from_IWICMetadataBlockReader(iface);
+if (!pcCount) return E_INVALIDARG;
+*pcCount = This-ref;
+return S_OK;
}
static HRESULT WINAPI PngDecoder_Block_GetReaderByIndex(IWICMetadataBlockReader *iface,
Return reference count as block count?
Re: [patch 1/1] dlls/windowscodecs/pngformat.c: implemented 'PngDecoder_Block_GetCount'
On 10/05/2012 03:55 AM, Nikolay Sivov wrote:
On 10/5/2012 06:43, max+...@mtew.isa-geek.net wrote:
From: Max TenEyck Woodbury max+...@mtew.isa-geek.net
---
dlls/windowscodecs/pngformat.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/windowscodecs/pngformat.c
b/dlls/windowscodecs/pngformat.c
index 686f9c6..e8e7cbe 100644
--- a/dlls/windowscodecs/pngformat.c
+++ b/dlls/windowscodecs/pngformat.c
@@ -899,8 +899,10 @@ static HRESULT WINAPI
PngDecoder_Block_GetContainerFormat(IWICMetadataBlockReade
static HRESULT WINAPI
PngDecoder_Block_GetCount(IWICMetadataBlockReader *iface,
UINT *pcCount)
{
-FIXME(%p,%p: stub\n, iface, pcCount);
-return E_NOTIMPL;
+PngDecoder *This = impl_from_IWICMetadataBlockReader(iface);
+if (!pcCount) return E_INVALIDARG;
+*pcCount = This-ref;
+return S_OK;
}
static HRESULT WINAPI
PngDecoder_Block_GetReaderByIndex(IWICMetadataBlockReader *iface,
Return reference count as block count?
I've screwed up. This is NOT what is supposed to be returned here. I am
in the process of digging out what this really should be. Please do
NOT apply this patch! (However, it does trigger at least one app to
do something useful and reveals the need for other changes...)
Re: [PATCH 1/3] ntoskrnl.exe: Add include/ddk/ntifs.h header with EPROCESS struct definition.
Did you check the DDK where it's supposed to be declared? -- Dmitry.
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
Christian Costa titan.co...@gmail.com wrote:
+EPROCESS process_info;
+
#ifdef __i386__
#define DEFINE_FASTCALL1_ENTRYPOINT( name ) \
__ASM_STDCALL_FUNC( name, 4, \
@@ -1200,8 +1203,11 @@ NTSTATUS WINAPI FsRtlRegisterUncProvider(PHANDLE
MupHandle, PUNICODE_STRING Redi
*/
PEPROCESS WINAPI IoGetCurrentProcess(void)
{
-FIXME(() stub\n);
-return NULL;
+FIXME((): partial stub\n);
+
+process_info.UniqueProcessId = (PVOID)PsGetCurrentProcessId();
+
+return process_info;
}
Why do you think that returning the structure filled with garbage is better
than returning NULL?
--
Dmitry.
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
2012/10/5 Dmitry Timoshkov dmi...@baikal.ru
Christian Costa titan.co...@gmail.com wrote:
+EPROCESS process_info;
+
#ifdef __i386__
#define DEFINE_FASTCALL1_ENTRYPOINT( name ) \
__ASM_STDCALL_FUNC( name, 4, \
@@ -1200,8 +1203,11 @@ NTSTATUS WINAPI FsRtlRegisterUncProvider(PHANDLE
MupHandle, PUNICODE_STRING Redi
*/
PEPROCESS WINAPI IoGetCurrentProcess(void)
{
-FIXME(() stub\n);
-return NULL;
+FIXME((): partial stub\n);
+
+process_info.UniqueProcessId = (PVOID)PsGetCurrentProcessId();
+
+return process_info;
}
Why do you think that returning the structure filled with garbage is better
than returning NULL?
It is not supposed to return NULL afaik but a valid pointer as you said
before.
The structure is zeroed except UniqueProcessId I need for MDL functions.
MDL struct have a PEPROCESS field and I would like to do things in a clean
way.
What's the problem with that ?
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
Christian Costa titan.co...@gmail.com wrote: Why do you think that returning the structure filled with garbage is better than returning NULL? It is not supposed to return NULL afaik but a valid pointer as you said before. The structure is zeroed except UniqueProcessId I need for MDL functions. MDL struct have a PEPROCESS field and I would like to do things in a clean way. What's the problem with that ? The problem is that the returned info in the structure must be valid, you can't initialize single field and pretend as done with it. -- Dmitry.
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
2012/10/5 Dmitry Timoshkov dmi...@baikal.ru Christian Costa titan.co...@gmail.com wrote: Why do you think that returning the structure filled with garbage is better than returning NULL? It is not supposed to return NULL afaik but a valid pointer as you said before. The structure is zeroed except UniqueProcessId I need for MDL functions. MDL struct have a PEPROCESS field and I would like to do things in a clean way. What's the problem with that ? The problem is that the returned info in the structure must be valid, you can't initialize single field and pretend as done with it. This struct is huge. Do you have some fields in mind? What matters is what drivers need. I can add some typical fields if needed but that could be done in other patches when needed as well.
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
Christian Costa titan.co...@gmail.com wrote: This struct is huge. Do you have some fields in mind? What matters is what drivers need. I can add some typical fields if needed but that could be done in other patches when needed as well. There are basic things like the header and object list management, besides things like ActiveProcessors, Affinity, BasePriority is not hard to fill from the start. Probably you need to duscuss how this should be done, something tells me that without server support this is not going to work very well. -- Dmitry.
Re: [PATCH 1/3] ntoskrnl.exe: Add include/ddk/ntifs.h header with EPROCESS struct definition.
On Friday 05 October 2012 10:00:00 am Christian Costa wrote: --- include/ddk/ntifs.h | 555 +++ 1 file changed, 555 insertions(+) create mode 100644 include/ddk/ntifs.h diff --git a/include/ddk/ntifs.h b/include/ddk/ntifs.h new file mode 100644 [...] What version of Windows were these extracted from?
Re: [PATCH 1/3] ntoskrnl.exe: Add include/ddk/ntifs.h header with EPROCESS struct definition.
2012/10/5 Paul Chitescu pa...@voip.null.ro On Friday 05 October 2012 10:00:00 am Christian Costa wrote: --- include/ddk/ntifs.h | 555 +++ 1 file changed, 555 insertions(+) create mode 100644 include/ddk/ntifs.h diff --git a/include/ddk/ntifs.h b/include/ddk/ntifs.h new file mode 100644 [...] What version of Windows were these extracted from? It's Vista. I didn't take these declarations directly from the ddk but on several sources on the web. I've just downloaded the DDK 7.1.0 to verify and make some changes if needed.
Re: [PATCH 1/3] ntoskrnl.exe: Add include/ddk/ntifs.h header with EPROCESS struct definition.
2012/10/5 Christian Costa titan.co...@gmail.com 2012/10/5 Paul Chitescu pa...@voip.null.ro On Friday 05 October 2012 10:00:00 am Christian Costa wrote: --- include/ddk/ntifs.h | 555 +++ 1 file changed, 555 insertions(+) create mode 100644 include/ddk/ntifs.h diff --git a/include/ddk/ntifs.h b/include/ddk/ntifs.h new file mode 100644 [...] What version of Windows were these extracted from? It's Vista. I didn't take these declarations directly from the ddk but on several sources on the web. I've just downloaded the DDK 7.1.0 to verify and make some changes if needed. I cannot find these definitions in ddk 7.1.0 headers. It does not seem they are supposed to be in the DDK. I based my patch on these ones at http://www.nirsoft.net/kernel_struct/vista/EPROCESS.html. I saw on the web that ntifs.h was always involved.
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
2012/10/5 Dmitry Timoshkov dmi...@baikal.ru Christian Costa titan.co...@gmail.com wrote: This struct is huge. Do you have some fields in mind? What matters is what drivers need. I can add some typical fields if needed but that could be done in other patches when needed as well. There are basic things like the header and object list management, besides things like ActiveProcessors, Affinity, BasePriority is not hard to fill from the start. Probably you need to duscuss how this should be done, something tells me that without server support this is not going to work very well. What do you mean by object list management ? There is only one element for now : nothing before nothing after. Unless lists are circular. I'm open for discussion but it's hard without an idea of what drivers do and what we want to support since wine is not intended to run all driver types. So at the beginning, maybe we can do something simple and improve the infrastructure as needs appear.
Re: 'PngDecoder_Block_GetCount'
Hmm. I definitely misunderstood what this function was intended to do. As I understand things now, there are two ways to approach processing graphical information: 1) As a stream of information to be processed in the order it arrives. 2) As an aggregate to be processed all at once. The PNG format is specifically designed to allow it to be processed as a stream with good efficiency. Processing the aggregate is done by processing the stream and accumulating the aggregated information. The key interface to the stream approach is to enumerate the blocks and process each one as you come to it. This approach has already been implemented. The strategy of the aggregate approach is to have an array of blocks and use an index to access the elements of the array. The key elements are getting the number of blocks (this function) and getting access to the block by its index number (using 'GetReaderByIndex'). This approach has not been implemented. To do this efficiently, it should be possible to validate the correctness of each block WITHOUT getting into any more details about the blocks than possible. Have I got this right so far?
Help getting amd64 assembly patch into wine?
Dan, I found your message very unclear. The patch adds support for OpenMP programs like this: And then you start talking about vcomp_fork without telling us where it comes from and what it should do. So I'll guess from the names. - vcomp_for_static/sections_init sound like startup code initialisation of BSS and DATA segments. - fork sounds like running code with thread-local copies to BSS and DATA I guess vcomp eats up one register or parameter to keep a pointer to the thread-local storage. p_vcomp_fork(0, 1, _test_vcomp_fork_worker1, ncalls); Your code does not explain what the first parameter is. I believe that va_list etc. is not going to lead you anywhere. First, va_* is only one side. E.g. include/wine/test.h uses va_list and __builtin_ms_va_list You need the other side too, namely, given a va_* structure, call a function X with the parameters given in that structure. The GNU ffcall library http://www.gnu.org/software/libffcall/ distinguishes both. One half is called vacall, the other one avcall :-) Your tests show that you need 2 different va_lists: p_vcomp_fork(0, 5, _test_vcomp_fork_worker5, 1, 2, 3, 4, 5); 1. On the receiver side, va_start would create a va_list for the parameter set (0, 5, function pointer, 1, 2, 3, 4, 5) 2. But now you need to call something with the apparent parameter set (1, 2, 3, 4, 5) (I say apparent because you don't explain the vcomp execution model. Is a hidden parameter added somewhere? What's that first parameter 0 to fork?) Obviously, if you don't know the internals of a va_list, you'll not be able to transform one structure into the other. So bad, now what is actually needed? Think assembly. Using your test example: +p_vcomp_fork(0, 5, _test_worker5, 1, 2, 3, 4, 5); _vcomp_fork finds data: 1. on the stack 2. in registers 3. in FP registers The C stack layout is (topmost first): 0, then 5, _test_worker5, 1, 2, etc. What you obviously need to do is, supposing the first parameters are duplicated in registers: - Save registers and possibly FP registers into a structure - Remember the stack pointer and number of elements, -- hereby creating a va_args structure whose layout you know. - CreateThread?? etc. - Restore registers - Shift register parameters by 3 items: (assuming 5 such registers) register-for-param-1 (1) - register-for-param-4 register-for-param-2 (2) - register-for-param-5 - Copy from stack into registers register-for-param-3 (3) - from stack register-for-param-4 (4) - from stack register-for-param-5 (5) - from stack - POP 0 and 5 - POP _test_worker5 into scratch register - JMPTO _test_worker5 via scratch register This assumes there's nothing like CreateThread so you can run from the original stack. If there were, you'd need to copy the 5 elements from the stack to the new one (which is presumably why vcomp_fork receives their number as parameter). Does this help? Jörg Höhle
Re: [patch 1/1] dlls/windowscodecs/pngformat.c: implemented 'PngDecoder_Block_GetCount'
You should take a look at the PNG format spec, particularly the part about chunks: http://www.libpng.org/pub/png/spec/1.2/PNG-Structure.html I believe what's needed here is to return all ancillary chunks.
Re: Help getting amd64 assembly patch into wine?
On Fri, Oct 5, 2012 at 7:27 AM, joerg-cyril.hoe...@t-systems.com wrote: I found your message very unclear. The patch adds support for OpenMP programs like this: And then you start talking about vcomp_fork without telling us where it comes from and what it should do. Good point - it's unfair to expect people to run Visual C and look at its .cod / .asm listing files as my message suggests. I'll document the vcomp execution model better in my next draft. So I'll guess from the names. - vcomp_for_static/sections_init sound like startup code initialisation of BSS and DATA segments. No, those are called at the start of a new parallel section by every participating thread. They set up thread-local stuff. - fork sounds like running code with thread-local copies to BSS and DATA I guess vcomp eats up one register or parameter to keep a pointer to the thread-local storage. It means 'Run this helper function on all cores, and pass it these parameters, which are all pointers to local variables'. p_vcomp_fork(0, 1, _test_vcomp_fork_worker1, ncalls); Your code does not explain what the first parameter is. It's a boolean saying whether to actually run in parallel, or just in the current thread. I believe that va_list etc. is not going to lead you anywhere. I'm going to give Maarten's suggestion a shot. If it works, the only assembly left will be a nearly verbatim copy of code in oleaut32/typelib.c Does this help? Yes, your questions are very helpful. I'll run the next draft by both you and Maarten if you don't mind. - Dan
Re: Help getting amd64 assembly patch into wine?
On Fri, Oct 05, 2012 at 04:27:24PM +0200, joerg-cyril.hoe...@t-systems.com wrote: So bad, now what is actually needed? Think assembly. Using your test example: +p_vcomp_fork(0, 5, _test_worker5, 1, 2, 3, 4, 5); _vcomp_fork finds data: 1. on the stack 2. in registers 3. in FP registers The C stack layout is (topmost first): 0, then 5, _test_worker5, 1, 2, etc. That sort of stack layout is likely to be valid for i386, but not for amd64. 64-bit (amd64) windows uses a different calling convention from (almost) every one else. But I don't think you can convert a partially consumed va_list back into an argument list (eg to delete an initail argument) without 'cheating'. On linux (and probably windows) the first int/ptr args are passed in integer registers and the first few FP args are passed in FP registers. When the registers run out, values are stacked. This means that these two calls are equivalent: printf(int %d, fp %f\n, 2, 3.15159); printf(int %d, fp %f\n, 3.15159, 2); the va_arg() processing has to remember which registers have already been processed in order to know where to find the next argument. David -- David Laight: da...@l8s.co.uk
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
* On Fri, 5 Oct 2012, Christian Costa wrote: 2012/10/5 Dmitry Timoshkov dmi...@baikal.ru What matters is what drivers need. I can add some typical fields if needed but that could be done in other patches when needed as well. There are basic things like the header and object list management, besides things like ActiveProcessors, Affinity, BasePriority is not hard to fill from the start. Probably you need to duscuss how this should be done, something tells me that without server support this is not going to work very well. What do you mean by object list management ?There is only one element for now : nothing before nothing after. Unless lists are circular. I'm open for discussion but it's hard without an idea of what drivers do and what we want to support since wine is not intended to run all driver types. So at the beginning, maybe we can do something simple and improve the infrastructure as needs appear. This is nth time the discussion drives to the question: How do you test ntoskrnl co functionality, folks? I suppose Christian debugs some application which loads own, custom sys-drivers. Wine has no tests which would build / load some simple sys-driver; and that needs to change in future, I'd say. Well, this topic already was brought in once by Damjan Jovanovic. [1] Plus, there are guys compiling kernel mode drivers with MinGW(-64) already: [2][3][4] Or am I misunderstanding the right way to go? S. [1] http://www.winehq.org/pipermail/wine-devel/2010-March/082460.html [2] http://strdup.livejournal.com/34596.html [3] http://sourceforge.net/projects/mingw-w64/forums/forum/723797/topic/3163052 [4] http://www.fccps.cz/download/adv/frr/win32_ddk_mingw/win32_ddk_mingw.html
Re: [PATCH 2/3] ntoskrnl.exe: Improve IoGetCurrentProcess stub.
Le 05/10/2012 23:01, Saulius Krasuckas a écrit : * On Fri, 5 Oct 2012, Christian Costa wrote: 2012/10/5 Dmitry Timoshkov dmi...@baikal.ru What matters is what drivers need. I can add some typical fields if needed but that could be done in other patches when needed as well. There are basic things like the header and object list management, besides things like ActiveProcessors, Affinity, BasePriority is not hard to fill from the start. Probably you need to duscuss how this should be done, something tells me that without server support this is not going to work very well. What do you mean by object list management ?There is only one element for now : nothing before nothing after. Unless lists are circular. I'm open for discussion but it's hard without an idea of what drivers do and what we want to support since wine is not intended to run all driver types. So at the beginning, maybe we can do something simple and improve the infrastructure as needs appear. This is nth time the discussion drives to the question: How do you test ntoskrnl co functionality, folks? I suppose Christian debugs some application which loads own, custom sys-drivers. Wine has no tests which would build / load some simple sys-driver; and that needs to change in future, I'd say. Well, this topic already was brought in once by Damjan Jovanovic. [1] Plus, there are guys compiling kernel mode drivers with MinGW(-64) already: [2][3][4] Or am I misunderstanding the right way to go? S. [1] http://www.winehq.org/pipermail/wine-devel/2010-March/082460.html [2] http://strdup.livejournal.com/34596.html [3] http://sourceforge.net/projects/mingw-w64/forums/forum/723797/topic/3163052 [4] http://www.fccps.cz/download/adv/frr/win32_ddk_mingw/win32_ddk_mingw.html Right. Mainly game and their copy protection stuff. Having a testing environment would be great. It's hard to test things. Thanks for the links.
Re: 'PngDecoder_Block_GetCount'
On 10/05/2012 10:58 AM, Vincent Povirk wrote: You should take a look at the PNG format spec, particularly the part about chunks: http://www.libpng.org/pub/png/spec/1.2/PNG-Structure.html I believe what's needed here is to return all ancillary chunks. First, thank you for the pointer to the HTML version of the PNG spec. I had the PDF one already, but the cross references make it a lot easier to use. Beyond that, I am not sure what you are talking about. The value to be returned (at the address specified by the 2nd parameter) is an unsigned integer. There is no place to return 'chunks'; only a place to return a count. In order to get that count, each and every one of the chunks needs to be scanned and validated. During the scanning process, it would make sense to store the location of chunks in an indexed data structure. That would nominally be an array, but a linked list or tree would also work. For a general WIC design, there needs to be at least three layers with indexes built into at least two of the layers. The top layer is the entire file. The next layer is the set of frames in the file. PNG only allows one frame per file, but other formats can have several (even many) frames. The next layer is the set of blocks (in this case chunks) in each frame. In some cases there might be a need for additional layers. All this means I need to dig out the 'class' structure of IWIC, but it would help if someone can confirm that I have not gone off the deep end yet.
Re: 'PngDecoder_Block_GetCount'
Beyond that, I am not sure what you are talking about. The value to be returned (at the address specified by the 2nd parameter) is an unsigned integer. There is no place to return 'chunks'; only a place to return a count. Other methods on that interface allow you to return an IWICMetadataReader, which need to be based on individual PNG chunks. Obviously, the GetCount method just gives the number that can be returned. In order to get that count, each and every one of the chunks needs to be scanned and validated. During the scanning process, it would make sense to store the location of chunks in an indexed data structure. That would nominally be an array, but a linked list or tree would also work. I don't think it's really necessary to verify the checksum. You just need to read a few bytes of header to get the chunk type and the location of the next one. See the png_read_chunk function. I don't understand anything you said after that paragraph.
Re: 'PngDecoder_Block_GetCount'
On 10/05/2012 05:53 PM, Vincent Povirk wrote: Beyond that, I am not sure what you are talking about. The value to be returned (at the address specified by the 2nd parameter) is an unsigned integer. There is no place to return 'chunks'; only a place to return a count. Other methods on that interface allow you to return an IWICMetadataReader, which need to be based on individual PNG chunks. Obviously, the GetCount method just gives the number that can be returned. Agreed then. This is only one part of the total aggregate interface. If this routine is implemented, the other methods need to be done as well. The enumeration of the chunks needs to be recorded. In order to get that count, each and every one of the chunks needs to be scanned and validated. During the scanning process, it would make sense to store the location of chunks in an indexed data structure. That would nominally be an array, but a linked list or tree would also work. I don't think it's really necessary to verify the checksum. You just need to read a few bytes of header to get the chunk type and the location of the next one. See the png_read_chunk function. I think the CRC check would be a good idea. As the rational in the spec mentioned, doing it guards against transition (and other) errors. cynic_modeNo assurance that Murphy (or his so unkind worshipers) can't get in of course./cynic_mode I don't understand anything you said after that paragraph. I was thinking about other formats the IWIC framework has to handle. Not just about PNG.
Re: wined3d: recognize Radeon HD 6670
On 5 October 2012 22:12, Austin English austinengl...@gmail.com wrote: Fixes http://bugs.winehq.org/show_bug.cgi?id=31891 That doesn't look right (and doesn't do what you think it does anyway), did you verify this against the Windows driver? It probably just needs an entry to map HD 6670 to CARD_AMD_RADEON_HD6600, although it's a bit curious that most of the existing entries for NI have generic strings like HD 6600.
Wine test bot
Hi, Is there a problem with Wine test bost? Jobs seem to be stuck and some VMs show problem of memory. Christian
