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 time zone parsing for "*_Date" attributes (#52) #54

Merged
merged 2 commits into from
May 7, 2021

Conversation

rlue
Copy link
Contributor

@rlue rlue commented May 5, 2021

Prior to this commit,
this library would apply the local system time zone to UTC timestamps.

For instance,

<Encoded_Date>UTC 2018-03-30 12:12:08</Encoded_Date>

would produce

>> MediaInfo.from(file).video.encoded_date
=> 2018-03-30 12:12:08 -0700

This commit addresses this problem
by moving the "UTC" prefix to the end of the string before parsing it.

Prior to this commit,
this library would apply the local system time zone to UTC timestamps.

For instance,

    <Encoded_Date>UTC 2018-03-30 12:12:08</Encoded_Date>

would produce

    >> MediaInfo.from(file).video.encoded_date
    => 2018-03-30 12:12:08 -0700

This commit addresses this problem
by moving the "UTC" prefix to the end of the string before parsing it.
@rlue
Copy link
Contributor Author

rlue commented May 5, 2021

This commit fixes #52.

A separate issue?

For the sake of clarity, it also adds two unimplemented spec examples for a separate-but-related issue, which can be summarized thusly:

  • The mediainfo CLI utility reports some timestamps as UTC, some with no zone info at all.
  • Previously, this mediainfo ruby gem would always convert those timestamps to the local system zone (without adding the appropriate UTC offset).
  • This commit corrects the gem's parsing of UTC timestamps, but...
  • in most cases, there is no way to fix parsing for zoneless timestamps.

Consider the metadata provided in /spec/fixtures/xml/trailing_zero.xml:

<media ref="Desktop/test.MOV">
  <track type="General">
    <File_Modified_Date>UTC 2018-03-30 12:12:08</File_Modified_Date>
    <File_Modified_Date_Local>2018-03-30 08:12:08</File_Modified_Date_Local>
  </track>
</media>

It's reasonable to assume here that File_Modified_Date and File_Modified_Date_Local should represent the same time in different zones (i.e., 2018-03-30 12:12:08 UTC & 2018-03-30 08:12:08 -0400). However, the mediainfo ruby gem will report File_Modified_Date_Local in the local system time zone (e.g., 2018-03-30 08:12:08 -0700) instead.

What to do?

First off, let's be clear that this problem was not introduced by this PR. Rather, the changes in this PR have merely solved half of a problem: previously, both kinds of timestamps were broken; now, only one is.

I don't think there's a clean solution, though. The only reliable to way infer what the value of File_Modified_Date_Local should be is to compare it to File_Modified_Date, and I don't think it's safe to assume that every zoneless timestamp will have a corresponding UTC one.

It's not ideal, but I think this problem should either be ignored entirely, or mediainfo should emit a warning that local timestamps are likely not in the correct zone.

Thoughts?

@NorseGaud NorseGaud self-requested a review May 5, 2021 01:28
@NorseGaud
Copy link
Collaborator

Should this change and the second comment/issue be mentioned in the README?

@rlue
Copy link
Contributor Author

rlue commented May 5, 2021

Yes, that is a better idea! How's this?

@rlue
Copy link
Contributor Author

rlue commented May 5, 2021

For the record, based on the research I've done, there is no universal standard for timestamps in video/audio file metadata, but ISO 8601 comes pretty close.

I checked a few files media files on my own computer (.mp4, .mov, .m4a) and they all stored the timestamp data as UTC. I even used ffmpeg to try to set a non-UTC timestamp on a video file, and ffmpeg converted it to UTC before saving (i.e., it added seven hours to the time and added the UTC label).

Note that the timestamp may still be incorrect for other reasons. For instance, the .mov file I linked to in #52 was taken on a digital camera that I have to manually set the time on. I set it to local time, then took a video at 15:48 local time, and it was recorded as 15:48 UTC. This is not a bug in mediainfo; it's a flaw in the design of the camera.

Copy link
Collaborator

@NorseGaud NorseGaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@NorseGaud NorseGaud merged commit c403bb4 into greatseth:master May 7, 2021
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.

None yet

2 participants