search

w3c / tvcontrol-api

TV Control API specification - https://w3c.github.io/tvcontrol-api/
10 stars 11 forks source link

Use DOMTimeStamp type for date and time values #40

Closed chrisn closed 7 years ago

chrisn commented 7 years ago

See #5. This pull request replaces use of unsigned long long with DOMTimeStamp.

paulhiggs commented 7 years ago

Did you really intend to do this change on "readonly attribute unsigned long long duration" as well? The duration attribute is defined as program duration in milliseconds, not a date/time value (even if the underlying types are the same) so according to section 3.3 of the TAC API Design Principles, we should not use this type for durations.

chrisn commented 7 years ago

I wondered about that, and reading the DOMTimeStamp definition here I figured that it could be used for durations, as a relative amount of time, despite the name. I interpreted the TAG guidance as specifying the epoch to use for absolute times. But I agree it doesn't seem quite right.

paulhiggs commented 7 years ago

I agree that the definition of DOMTimeStamp says it can be used for Relative Time (interval==duration) but the W3 API spec seems to disallow this. I would have expected something in section 3.2 of the TAC API Design Principles for this.

Also, by using DOMTimeStamp (instead of unsigned long long) will we ever run into problems with the HTMLMediaElement attributes...

  // playback state
           attribute double currentTime;
  readonly attribute unrestricted double duration;

especially when it comes to recording something from the current point in the Media Element?

paulhiggs commented 7 years ago

@stevem-tw, the merge & commit for the description of duration is not correct. It now says

  • MUST return the duration of the TV/Radio program, in milliseconds
  • relative to 1970-01-01T00:00:00Z.

durations are not relative to an epoch, so the description for duration remove the "relative to..." qualifier.

stevem-tw commented 7 years ago

Thanks for pointing this out, Paul. I've corrected my local copy and will be pushing this out soon.

chrisn commented 7 years ago

Thanks @stevem-tw for merging this, and sorry for the copy/paste error. I agree now with Paul's previous comment, and I'd recommend still using unsigned long long for durations. Paul also made another good point here about currentTime that we should capture in a separate issue.