Skip to content
This repository has been archived by the owner on Oct 4, 2020. It is now read-only.

invalid timestamps on windows #117

Open
rokups opened this issue Jun 7, 2016 · 3 comments
Open

invalid timestamps on windows #117

rokups opened this issue Jun 7, 2016 · 3 comments
Labels
windows Just kill me now.

Comments

@rokups
Copy link

rokups commented Jun 7, 2016

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..

@SamWhited
Copy link
Member

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.

@rokups
Copy link
Author

rokups commented Jun 7, 2016

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.

@campaul
Copy link
Member

campaul commented Jun 7, 2016

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.

@SamWhited SamWhited added the windows Just kill me now. label Sep 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
windows Just kill me now.
Projects
None yet
Development

No branches or pull requests

3 participants