rawkit icon indicating copy to clipboard operation
rawkit copied to clipboard

invalid timestamps on windows

Open rokups opened this issue 8 years ago • 3 comments

On windows invalid timestamp in metadata is returned. I tracked it down and it turns out struct_16 and struct_17 have invalid timestamp type - it should be c_uint64. As a consequence remaining fields are also corrupted. This is very unfortunate as time_t size is not defined in standard and it varies platform by platform. We can even define _USE_32BIT_TIME_T on windows and make time_t 32 bit integer. Not sure how this could be fixed in a reliable way..

rokups avatar Jun 07 '16 14:06 rokups

Thanks for the report! Contentious discussion to follow: :)

@campaul This begs the broader question: Do we care about Windows users at all? I don't, but you may have different opinions depending on where you want Photoshell to run. If we can ensure this works on *nix like systems, I think that should be enough.

SamWhited avatar Jun 07 '16 14:06 SamWhited

My wife is professional photographer so she made me throw together some scripts for her workflow. Since she is windows user here i am.. ;)

If you do not care for rock-solid windows support i suppose you could do like c_uint64 if os.name == 'nt' else c_uint. That should get it working in most cases as on recent windows platforms time_t is usually 64 bit (even if application is 32 bit). Anything else gets to use 32 bit integer, just like it is now.

rokups avatar Jun 07 '16 14:06 rokups

We don't have the time to support Windows ourselves, but I'm not going to turn down pull requests that make rawkit work in more places. Also this issue isn't specifically about Windows. The size of time_t is going to vary between other platforms too and there is no guarantee of what its size will be.

What's further puzzling is on 64-bit Linux systems it should be a 64-bit long int, but we have it set to unsigned integer and it's still working.

I'm leaning towards providing some sane defaults (once I figure out what those actually are) with a simple way to override the size of the timestamp field if you're on a system doing something unexpected.

campaul avatar Jun 07 '16 15:06 campaul