Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix date offset application for videos #409

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dirextric
Copy link

While using Elodie to sort some videos, I found that it would consistently move videos' time taken by the same amount as my timezone's offset from UTC. This caused some videos to be placed in the wrong folder.

I noticed that video.py has some code dedicated to calculating an date offset, but that this offset is never applied because of some missing = signs. I assume that this is an oversight since the affected code doesn't really do anything at the moment.

Adding the = signs fixes the issue for me, and Elodie now correctly sorts my files.

Make sure the offset is actually applied to seconds_since_epoch so that timezone is taken into account.
@CLAassistant
Copy link

CLAassistant commented Jun 19, 2021

CLA assistant check
All committers have signed the CLA.

@jmathai
Copy link
Owner

jmathai commented Jun 19, 2021

That’s wild. Thanks for catching this....I assume there weren’t any unit tests covering proper times on videos? Can we add that to this PR? If you aren’t familiar then I can help.

@dirextric
Copy link
Author

I looked at the tests, and apparently there is one (elodie.tests.media.video_test.test_get_date_taken) checking for proper time on a test video (video.mov). I don't know how it hasn't been failing, since Elodie also sorts that video wrong on my computer. With the changes in this PR, the test fails, but it seems the test video still isn't sorted correctly. My videos are sorted fine, though.

After some digging, I think what is happening is that if a video was taken in the same timezone Elodie is run in, the changes in this PR will correctly sort the video. However, if the video was taken in a different timezone, the video will be sorted wrong, with the amount of error being the difference in time between the video's timezone and the computer running Elodie's timezone.

This seems to be a result of the method being used to calculate date offset. The program uses time.mktime() to convert the time in the video's metadata into epoch time, but then uses time.gmtime() to convert the epoch time back into "normal" time. The issue is that time.mktime() takes its input as local time, while time.gmtime() converts its input into UTC time. This causes a disparity which I believe the date offset is supposed to neutralize. However, this doesn't work when the video's and computer's timezones are not the same.

I have some ideas on how to fix this, but before I go further, I have one clarifying question. What is the intended behavior here? What timezone should Elodie be using to sort videos? For example, the test video.mov was taken in a UTC-8 timezone. If the user is in a UTC-4 timezone, should Elodie sort the video using UTC-8 time, UTC-4 time, or plain UTC time?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants