FrameTimeCode should be identic to itself when applying the resulting values to create a new FrameTimeCode

Hi !

Thank you for PySceneDetect.

I ran into issues where after some serialization the resulting time frame was not the expected one.
For example:

frame_time_code = FrameTimecode(61, fps=14)
seconds = frame_time_code.get_seconds()
time_code = frame_time_code.get_timecode()

...

FrameTimecode(seconds, fps=14).get_frames() == 60 # not 61
FrameTimecode(time_code, fps=14).get_frames() == 60 # not 61

I found the corresponding code with flooring (with int instead of round) :

PySceneDetect/scenedetect/frame_timecode.py

Lines 227 to 236 in f6c61dd

    
               def _seconds_to_frames(self, seconds): 
        
                   # type: (float) -> int 
        
                   """ Converts the passed value seconds to the nearest number of frames using 
        
                   the current FrameTimecode object's FPS (self.framerate). 
        
                   Returns: 
        
                       Integer number of frames the passed number of seconds represents using 
        
                       the current FrameTimecode's framerate property. 
        
                   """ 
        
                   return int(seconds * self.framerate)

PySceneDetect/scenedetect/frame_timecode.py

Line 307 in f6c61dd

return int(secs * self.framerate)

I ran into the following code which seems to indicate this is on purpose. What is the reason?

PySceneDetect/tests/test_frame_timecode.py

Lines 114 to 116 in f6c61dd

    
           assert FrameTimecode(timecode='00:00:01', fps=1).frame_num == 1 
        
           assert FrameTimecode(timecode='00:00:01.9999', fps=1).frame_num == 1 
        
           assert FrameTimecode(timecode='00:00:02.0000', fps=1).frame_num == 2

Have a nice day !

@Wirg

Hey @Wirg;

Thanks for looking into this. It looks like some of the existing test cases are failing after your change (probably just a few off-by-one errors now), so will need to see why that is before merging #269 (thanks for the PR as well). I'm not sure if this should be taken for v0.5 or not, as this could possibly be a breaking change, but I'd be happy to consider it for v0.6.

I ended up doing a similar workaround in v0.6 for the PyAV backend, so I think your change is correct. Truthfully it's been quite a few years since I've touched FrameTimecode, so I can't really remember the intent behind these test cases you pointed out:

PySceneDetect/tests/test_frame_timecode.py

Lines 114 to 116 in f6c61dd

    
           assert FrameTimecode(timecode='00:00:01', fps=1).frame_num == 1 
        
           assert FrameTimecode(timecode='00:00:01.9999', fps=1).frame_num == 1 
        
           assert FrameTimecode(timecode='00:00:02.0000', fps=1).frame_num == 2

I may mark this as a known issue for v0.5, but bring your change in for v0.6. Thanks for the report!

[FrameTimecode] Change to use rounding instead of truncation. #268

@Wirg

I've accepted your suggested solution for v0.6 (c5ac42b) and it will be included in the next beta release, as well as v0.6. I'm reluctant to merge this fix into v0.5 as it may constitute a breaking change for some existing applications.

TODO before closing this issue:

Add identity unit tests as @Wirg suggested in fix: FrameTimeCode approximation flooring instead of rounding #269
Update v0.5 changelog marking this as a known issue

Thanks for the fast answer!

I think it's ok for me, I agree with your reasoning. I may try the v0.6 branch then.

This is now available in the latest release of v0.6 (v0.6-rc0), feel free to give it a try!

[docs] Update known issues for v0.5. #268

Navigation Menu

Repository navigation

FrameTimeCode should be identic to itself when applying the resulting values to create a new FrameTimeCode #268

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	def _seconds_to_frames(self, seconds):
	# type: (float) -> int
	""" Converts the passed value seconds to the nearest number of frames using
	the current FrameTimecode object's FPS (self.framerate).

	Returns:
	Integer number of frames the passed number of seconds represents using
	the current FrameTimecode's framerate property.
	"""
	return int(seconds * self.framerate)

	assert FrameTimecode(timecode='00:00:01', fps=1).frame_num == 1
	assert FrameTimecode(timecode='00:00:01.9999', fps=1).frame_num == 1
	assert FrameTimecode(timecode='00:00:02.0000', fps=1).frame_num == 2

FrameTimeCode should be identic to itself when applying the resulting values to create a new FrameTimeCode #268

Description

Activity

Breakthrough commented on Apr 20, 2022

Breakthrough commented on Apr 20, 2022

Wirg commented on Apr 21, 2022

Breakthrough commented on Apr 27, 2022

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions