ES-Alexander / pythonic-cv

Performant pythonic wrapper of unnecessarily painful opencv functionality
MIT License
41 stars 2 forks source link

Improve the format of the `timestamp` property to conform to "hh:mm:ss.sss" #19

Closed MatusGasparik closed 2 years ago

MatusGasparik commented 2 years ago

Currently, if I call timestamp on VideoReader instance, say somewhere after the minute 8, I get the following output:

'8.0:15.320'

whereas it would be ideal to have it in the format:

'08:15.320'

...as it would conform to the ISO 8601 standard of "HH:MM:SS.SSS" and would be a lot easier to read when used in filenames.

These would be the necessary changes to fix in src/pcv/vidIO.py

@property
def timestamp(self):
    ''' Returns the video timestamp if possible, else 0.0.

    Returns a human-readable time string, as hours:minutes:seconds.
    For the numerical ms value use self.get('timestamp') instead.

    self.timestamp -> str

    '''
    # cv2.VideoCapture returns ms timestamp -> convert to meaningful time
    seconds          = self.get('timestamp') / 1000
    minutes, seconds = divmod(seconds, 60)
    hours, minutes   = divmod(minutes, 60)
    if hours:
        return f'{int(hours):02d}:{int(minutes):02d}:{seconds:06.3f}'
    if minutes:
        return f'{int(minutes):02d}:{seconds:06.3f}'
    return f'{seconds:06.3f}s'
ES-Alexander commented 2 years ago

Good pickup :-) I’d note that round is preferred here over the truncating behaviour of int, particularly given the nature of floating point error.

Feel free to submit a PR. I’m in bed so can’t make modifications here at the moment.

ES-Alexander commented 2 years ago

Just had a proper look at this. I agree that zero-padding the times makes sense, for both consistency and clarity, but I prefer the existing structure of "fewer numbers means no big ones" rather than the ISO 8601 specification that "fewer numbers means less precision". Since ISO-8601 is intended to apply to clock times rather than durations I think that's acceptable.

As a compromise, I've added a separate property iso_timestamp that gives the timestamp in a valid ISO 8601 duration, as PThh:mm:ss.sss. I'll try to get a release out this weekend with these latest updates :-)

MatusGasparik commented 2 years ago

Ok. I'll submit a PR as soon as I'll have the chance.

I hope your being forced to stay in bed is nothing serious. In any case, I hope you'll be fit again in no time...

Cheers!

ES-Alexander @.***> schrieb am Fr., 15. Okt. 2021, 16:41:

Good pickup :-) I’d note that round is preferred here over the truncating behaviour of int, particularly given the nature of floating point error.

Feel free to submit a PR. I’m in bed so can’t make modifications here at the moment.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ES-Alexander/pythonic-cv/issues/19#issuecomment-944356575, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADS4QMWNWNGPOBUWTO5SZT3UHA4QVANCNFSM5GCCO6ZQ .

ES-Alexander commented 2 years ago

I ended up out the timestamps earlier today, so it’s all good for now. I also added you as a contributor in the README since you’ve made multiple helpful recommendations :-)

Haha, not forced to stay in bed - it was just late at night time and I was meant to be asleep (like now 😂)