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

StreamGenerator in tests assumes mvhd and mdhd contain the same timescale #1206

Closed
joeyparrish opened this issue Dec 22, 2017 · 2 comments
Closed
Labels
status: archived Archived and locked; will not be updated type: code health A code health issue
Milestone

Comments

@joeyparrish
Copy link
Member

StreamGenerator in our tests assumes mvhd and mdhd boxes contain the same timescale. This is true for the content we're templating on right now, but is not generally true. The mdhd box seems to override the mvhd box, so we should look at mdhd instead.

See #1191 (comment) for an example of content with different timescales in these two places.

This is not causing any issues right now, and is limited to a class used in our integration tests. But we should fix this to use the mdhd box to guard against breakage if/when we update or add to the segments used by StreamGenerator today.

@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Dec 22, 2017
@joeyparrish joeyparrish added this to the Backlog milestone Dec 22, 2017
@joeyparrish joeyparrish added the type: code health A code health issue label Mar 5, 2018
@joeyparrish joeyparrish modified the milestones: Backlog, v2.6 Mar 13, 2018
@joeyparrish joeyparrish removed the type: bug Something isn't working correctly label Jan 10, 2019
@joeyparrish
Copy link
Member Author

This is finally causing a bug in our tests.

StreamGenerator over our multidrm clip is now generating the wrong timestamps, which causes a buffering state in some percentage (maybe 2-5%?) of the offline integration test runs.

I'm not sure why the manifestation of this bug is so incredibly specific.

@joeyparrish
Copy link
Member Author

It's incredibly specific because it is the combination of this StreamGenerator bug, a very specific test expectation, and a very specific buffer starvation threshold that are causing the test to fail.

@joeyparrish joeyparrish modified the milestones: v2.6, v2.5 Apr 19, 2019
shaka-bot pushed a commit that referenced this issue Apr 22, 2019
Commit 1284dd4, Change-Id: I8cad9bfde3309de7c2b8301b90aa8c40b6e4d247
introduced a change to the buffering logic.  Instead of going into a
buffering state when less than 0.5 seconds of content was buffered,
the thredhold changed to one half of the configured rebufferingGoal.

Now the rule is one half of the rebufferingGoal or 0.5 seconds,
whichever is smaller.  This accounts for unusually small
rebufferingGoal settings, while keeping a saner default behavior.

This change in behavior was one part of the test flake described
in internal bug report b/128923302.  The other component to the test
flake was issue #1206, which still has a separate fix.

Issue #1206
Fixes b/128923302

Change-Id: I5f133959acac6fd4ebe7fed37d2df88892e6399e
@shaka-project shaka-project locked and limited conversation to collaborators Jun 21, 2019
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: code health A code health issue
Projects
None yet
Development

No branches or pull requests

2 participants