Increase test coverage for stream worker

[allenporter]

Proposed change

Increase test coverage of stream worker from ~80% to 92% in preparation for adding support for expiring nest urls.

Exercise the stream worker functionality by mocking av.open calls to return a
fake media container as well a fake decoded stream in the form of a series of
packets. Each test creates a packet sequence, with a mocked output buffer to capture the segments
pushed to the output streams. The packet sequence can be used to exercise
failure modes or corner cases like how out of order packets are handled.

This is in preparation for changing the stream behavior to support expiring urls, and
there is not a lot of existing test coverage. This test adds additional coverage for corner cases that is not possible to achieve from test_hls.py because this can create invalid streams that pyav cannot encode.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue: Nest SDM Camera stops streaming #42793
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

• No score or internal
• 🥈 Silver
• 🥇 Gold
• 🏆 Platinum

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[probot-home-assistant]

Hey there @hunterjm, @uvjustin, mind taking a look at this pull request as its been labeled with an integration (stream) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

[allenporter]

Based on discussion in home-assistant/architecture#482 it seems like these tests will still be useful. However, I think it might be a mistake to test these in terms of the Stream class directly rather than in terms of the "public" interfaces of the component. This will also help if we need to do any refactoring when introducing new abstractions.

[allenporter]

Thinking about it more, I think this approach may still have some value as (1) testing via the Stream class is close enough to the top level interface, and matches other tests and (2) I considered trying to exercise these corner cases using a similar strategy as test_hls.py however pyav won't let me encode videos in the format that these corner cases are exercising (e.g. out of order pts/dts, etc). Another approach to consider could be to refactor the code into simpler abstractions to let us test these corner cases independent of pyav (e.g. a code that just operates in terms of processing packets) however I think we'd want tests in place before considering that. So, I think this makes sense to review still.

[allenporter]

Swapping reviewers based on conversation in #45431

@uvjustin i'd like your take on if this is helpful. I worry about these subtle corner cases not being fully tested, and i'm pretty bad at manually testing things.

My only worry about this is it gets deep into the pyav details so the test harness may be more brittle than would be ideal. Another thought I have is that we can split the responsibilities of the worker a bit: into a few components to make unit testing in terms of packets easier. That is, imagine the worker is only decoding the stream and pushing packets into another class that handles all breaking into segments, verifying ordering, etc -- then we write these tests in terms of that interface that accepts streams of packets. (Though to refactor that, i'd want this test coverage first to verify it worked)

[uvjustin]

@allenporter I think this is very good - the tests capture/codify most of the problem workarounds we put in the worker (did you dig through the old issues to get these or were you able to deduce them from the code?) Even if we need to change these workarounds at least this harness will make clear that we are only changing what we intend to change.
We could refactor the worker as long as it doesn't add too much overhead. I don't think it's overly complex as is but maybe your refactor will still be cleaner.

[allenporter]

@allenporter I think this is very good - the tests capture/codify most of the problem workarounds we put in the worker (did you dig through the old issues to get these or were you able to deduce them from the code?) Even if we need to change these workarounds at least this harness will make clear that we are only changing what we intend to change.
We could refactor the worker as long as it doesn't add too much overhead. I don't think it's overly complex as is but maybe your refactor will still be cleaner.

Ok thank you for taking a look. I got these from the test coverage gaps / code, looking at the PRs and reading the previous issues, yes.

[uvjustin]

Maybe add one more test where we leave the dts in order but scramble up the pts to ensure that we are only enforcing proper ordering on the dts side?

[balloob]

@uvjustin don't forget to hit that approve button if you think it's ok 👍

[allenporter]

Looking good, I have a few more ideas to simplify further building one why you've done.

[uvjustin]

Looking good, I have a few more ideas to simplify further building one why you've done.

Ok, I'll wait for those.

[allenporter]

OK, this is ready for another look.

[uvjustin]

Just made a minor change to use some more realistic values for time_base. Does that look ok to you?

[allenporter]

Yeah, that looks good to me.

[uvjustin]

Looks good!

[uvjustin]

I had to add back PACKETS_PER_SEGMENT to fix the modulo checks as 1ddb138 ended up changing those inadvertently. I renamed PACKETS_TO_SEGMENTS just for consistency.

[allenporter]

OK, looks good. Want to also approve?