search

cta-wave / dpctf-tests

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

relax random_access_from_tolerance #167

Closed jpiesing closed 7 months ago

jpiesing commented 7 months ago

test-config.json defines "random_access_from_tolerance" as 200, i.e. 5 frames at 25Hz.

The description of random_access_from in the DPCTF spec says;

When the current play position reaches random_access_from initialize a seek to the value defined in random_access_to.

There are two reasons why 5 frames may not be enough.

  1. By the time the app detects that the current play position has reached random_access_from, the displayed frame may have moved on.
  2. The device likely has a media pipeline and initiating a seek may not empty this pipeline. It may not be possible to stop frames already in the pipeline being output

I recommend increasing random_access_from_tolerance from 5 to perhaps 10.

yanj-github commented 7 months ago

Thanks @jpiesing random_access_from_tolerance is defined in milliseconds. Number of allowed frame will be different depends on the content frame rate. Shall we change it from 200ms to 400ms?

jpiesing commented 7 months ago

Thanks @jpiesing random_access_from_tolerance is defined in milliseconds. Number of allowed frame will be different depends on the content frame rate. Shall we change it from 200ms to 400ms?

I'm not sure I understand the point. Yes, clearly the number of allowed frames will be different when the tolerance is defined in ms rather than frames. I suggested this because it was a configuration file change and did not require a change in the OF.

yanj-github commented 7 months ago

@jpiesing yes no OF code change required. What I was trying to ask is that you were recommend increasing random_access_from_tolerance from 5 to perhaps 10. 10 is 400ms in this case but it wont be 10 for different content group. Are we happy to make it 400ms, or calculate frame 10 to the smallest frame rate?

All different frame_rate for this test: 25, 30, 30000/1001

jpiesing commented 7 months ago

I propose changing the contents of test-config.json from;

./dpctf-tests/test-config.json: "random_access_from_tolerance": 200

to

./dpctf-tests/test-config.json: "random_access_from_tolerance": 400

I'm not proposing changing the units from ms to frames or allowing the combination of frames and ms as we have in some other places.

It's ms now so I see no particular reason to change the units to frames.

Apologies if I'm missing something.

yanj-github commented 7 months ago

This has been tested with one of the recording from Plugfest. @FritzHeiden can you now change "random_access_from_tolerance": 400 in ./dpctf-tests/test-config.json please?

FritzHeiden commented 7 months ago

Parameter updated with https://github.com/cta-wave/dpctf-tests/pull/170

yanj-github commented 7 months ago

Thanks @FritzHeiden, changes confirmed this can be closed. @jpiesing Feel free to close it.