cta-wave / dpctf-tests

Repo for DPCTF Tests. We prefer to keep the Tests separated from Test Runner
Other
1 stars 5 forks source link

current time check failed at the switching point #172

Closed yanj-github closed 2 months ago

yanj-github commented 2 months ago

After tested with relaxed tolerance https://github.com/cta-wave/device-playback-task-force/issues/123 1/framerate + 150, current time check is PASS for many tests however one or two failed at the switching point.

fullscreen-playback-of-switching-sets__ss1-1.html failed on 3 different devices:

fullscreen-playback-of-switching-sets__ss1-2.html failed on two:

overlapping-fragments__ss1.html also failed on two:

switching-set-playback__ss1-1.html failed on one:

switching-set-playback__ss1-2.html also failed on two:

yanj-github commented 2 months ago

Hi @jpiesing, I think the key issue is that, when we change tolerance from "2/framerate + 20" to "1/framerate + 150", it made the detected time diffs bigger. The cause of this it that it has relaxed the tolerance of comparing the diffs. However, it reduced the number of matching pairs of frames with the current time. I think best solution for now is that we keep the 1/framerate + 150 on the spec. But to make this relaxed for both matching pairs and comparing the diffs, OF code change required to take into account 150ms as well as the 1/framerate when searching for the matching pairs of frames with the current time.

jpiesing commented 2 months ago

Hi @jpiesing, I think the key issue is that, when we change tolerance from "2/framerate + 20" to "1/framerate + 150", it made the detected time diffs bigger. The cause of this it that it has relaxed the tolerance of comparing the diffs. However, it reduced the number of matching pairs of frames with the current time. I think best solution for now is that we keep the 1/framerate + 150 on the spec. But to make this relaxed for both matching pairs and comparing the diffs, OF code change required to take into account 150ms as well as the 1/framerate when searching for the matching pairs of frames with the current time.

Sorry but I don't understand the reference to matching pairs. This is a detail of how the OF works which I've never looked into. Is this included in one of the OF documents?

yanj-github commented 2 months ago

@jpiesing https://github.com/cta-wave/device-observation-framework/wiki/Observation-Algorithms#the-presented-sample-matches-the-one-reported-by-the-currenttime-value-within-the-tolerance-of-the-sample-duration It documented here to help you understand. OF uses sample_tolerance (1/frame) but not tolerance (150ms) at the moment when it searching for matching pairs.

yanj-github commented 2 months ago

@jpiesing as this is an implementation details on OF if there is no objection I will start making changes. I have tested on one TV for fullscreen-playback-of-switching-setsss1-1.html and fullscreen-playback-of-switching-setsss1-2.html it will resolve the issue.

jpiesing commented 2 months ago

@jpiesing as this is an implementation details on OF if there is no objection I will start making changes. I have tested on one TV for fullscreen-playback-of-switching-setsss1-1.html and fullscreen-playback-of-switching-setsss1-2.html it will resolve the issue.

Please go ahead.

yanj-github commented 2 months ago

This is working now for all the switching set tests. @FritzHeiden can you update "test-config.json" file to have new relaxed tolerance https://github.com/cta-wave/device-playback-task-force/issues/123 1/framerate + 150 please?

"tolerance": 150,
"frame_tolerance": 1,
yanj-github commented 2 months ago

I have made OF code change and now just waiting for tolerance to be changed in "test-config.json" file. This issue can be closed when "test-config.json" file updated.

louaybassbouss commented 2 months ago

tolerance is now change https://github.com/cta-wave/dpctf-tests/blob/master/test-config.json#L10 . @yanj-github please check and close issue if ok for you

yanj-github commented 2 months ago

Thanks @louaybassbouss changes confirmed. Closing.