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

new value for tolerance on playback duration #118

Closed
jpiesing opened this issue Sep 6, 2023 · 12 comments
Closed

new value for tolerance on playback duration #118

jpiesing opened this issue Sep 6, 2023 · 12 comments
Assignees
Labels
Blocker Issue must be resolved for V1 launch

Comments

@jpiesing
Copy link

jpiesing commented Sep 6, 2023

The current tolerance on the playback duration is 20ms.
This is 1/2 frame at 25Hz which seems too demanding given that errors accumulate between the video decoder, the panel, the camera and the OF. ​
I recommend it should be at least 40ms for 25Hz video​ but this needs to be validated by Fraunhofer from analysis of plugfest results​.

@yanj-github
Copy link
Contributor

yanj-github commented Sep 6, 2023

@jpiesing The current tolerance on the playback duration is 2 frames + 20ms as defined in test-config.json file.

"tolerance": 20,
"frame_tolerance": 2,

Wording from spec:
The presented sample shall match the one reported by the currentTime value within the tolerance of +/- (2/framerate + 20ms).

eg: for 25H which is 100ms and for 50H it is 60ms

@jpiesing
Copy link
Author

jpiesing commented Sep 6, 2023

@jpiesing The current tolerance on the playback duration is 2 frames + 20ms as defined in test-config.json file.

"tolerance": 20,
"frame_tolerance": 2,

Wording from spec: The presented sample shall match the one reported by the currentTime value within the tolerance of +/- (2/framerate + 20ms).

eg: for 25H which is 100ms and for 50H it is 60ms

I may be missing something but that's for the observation about currentTime, not for the duration observation.

image

vs

image

Also that's not what it says in the test report ....

image

@yanj-github
Copy link
Contributor

Sorry @jpiesing completly my bad I was looking at wrong value. Indeed they were for current Time match.
For durations I should have checked:

"duration_tolerance": 20,
"duration_frame_tolerance": 0,

Which are 0 frame with 20ms at the moment.

@louaybassbouss
Copy link
Collaborator

@jpiesing @yanj-github we will collect the number from the test results and then we can calculate the default tolerance for duration

@yanj-github
Copy link
Contributor

yanj-github commented Nov 15, 2023

@jpiesing and @louaybassbouss coming from the audio perspective this going to impact on audio as well.
We might need to apply same change to audio as well (after run audio duration observation on different devices), but are we happy to apply same value to audio?
At the moment tolerance is shared by both video and audio observations.
We can either:

  • Defer it now (initial release) for audio come back to this later (keep new tolerance same value as video)
  • Add separate parameters for audio with 20ms "audio_duration_tolerance": 20

#116 this issue I have same question as tx_mas shared for both video and audio.

@louaybassbouss
Copy link
Collaborator

@yanj-github I don't have preferences regarding for audio and video tolerance and I am fine to apply for now the same tolerance to video and audio.

@jpiesing
Copy link
Author

I would not expect audio to have the same tolerance as video as the reasons for the tolerance are somewhat specific to video frame rate conversions, from the encoded frame rate of the content to the frame rate of the display (panel) and from that to the frame rate of the camera.
I would assume for audio that there's a simpler process d->a at the output of the device being tested and a->d at the input to the camera. I'd further assume the only variation is perhaps a small difference in the clocks of the two devices.

@yanj-github
Copy link
Contributor

Thanks @louaybassbouss and @jpiesing for the comments.
For video we are capturing every 8.33ms (on 120fps) so there will be a difference between captured time vs where the frame actually appeared.
Audio is different, it is captured accurately with a little difference (couple of milliseconds).
We will need to run more tests on the device to work out what tolerance we need on audio duration check, but what I can see so far is that 20ms for audio duration passes most of the test.
I think we will need to separate out audio duration tolerance anyway. The question is that whether or not include it on the 1st release. It depends on whether or not audio is in scope plus the timescales.
Regarding to the issue #116, tx_mas is shared for both video and audio. Is different case than duration check. Start up delay is measured from where we detect initial play() status till the 1st detected audio sample. I think it is sensible to keep start up delay tolerance same for audio and video. We expect the starting delay be same otherwise audio and video not correctly synchronised. If one should delay more than the other, video behind audio or audio behind video, the A/V sync would fail.

@gitwjr
Copy link

gitwjr commented Nov 21, 2023

See @jpiesing comments in his post at https://standards.cta.tech/wg/dpctf/mail/thread/46870
The proposal is to change 20ms to 50ms for video. If no issues are raised. A new issue will be opened for audio.

@yanj-github
Copy link
Contributor

A new issue is opened for audio duration linked above. But not for maximum permitted start-up delay, I think it is better to leave tx_mas is shared for both video and audio.

@jpiesing
Copy link
Author

jpiesing commented Dec 4, 2023

@FritzHeiden Please can test-config.json be changed to 50ms as agreed above. Once you've done this, please close the issue.

@FritzHeiden
Copy link
Collaborator

Changed duration_tolerance in test-config.json from 20 to 50ms (#134)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker Issue must be resolved for V1 launch
Projects
None yet
Development

No branches or pull requests

5 participants