fix: Add null check for current reference

[willdharris]

Resolves #7174.

[shaka-bot]

Incremental code coverage: 100.00%

[joeyparrish]

I'm okay with the fix, but we have some consistent test failures. Looks like some expectations need to be updated.

[joeyparrish]

Or possibly some other test logic needs to be updated. Not sure.

[willdharris]

@joeyparrish Thanks for the review. I see the common failing test. I'll take a look at what is going on there.

[joeyparrish]

@willdharris, I have prepared new releases for v4.10.x and v4.9.x, but I will wait another day to see this get finished and merged.

[willdharris]

I'm still not sure how to resolve the failed test.
@avelad I see you looked into this one in the past. I'm sure it's been a while, but would you have any insight on this one?

shaka-player/test/media/streaming_engine_unit.js

Line 685 in c97e689

it('appends the ReadableStream data with low latency mode', async () => {

The failing test indicates appendBuffer is getting called 10x more than it was before the change with the simulated VOD asset in the test. Updating the expect does resolve the failed test and all others pass, but that 10x change seems excessive and worth looking in to.
I did run some smoke tests with our DASH Live content and the Low Latency assets from the demo page. With real content, I'm not seeing the excessive calls to appendBuffer like in the test. Maybe there is logic in the test that should be updated? I'm not very familiar with the LL feature or these tests, so it will take me more time to get up to speed.

[avelad]

I'm still not sure how to resolve the failed test. @avelad I see you looked into this one in the past. I'm sure it's been a while, but would you have any insight on this one?

shaka-player/test/media/streaming_engine_unit.js

Line 685 in c97e689

it('appends the ReadableStream data with low latency mode', async () => {

The failing test indicates appendBuffer is getting called 10x more than it was before the change with the simulated VOD asset in the test. Updating the expect does resolve the failed test and all others pass, but that 10x change seems excessive and worth looking in to. I did run some smoke tests with our DASH Live content and the Low Latency assets from the demo page. With real content, I'm not seeing the excessive calls to appendBuffer like in the test. Maybe there is logic in the test that should be updated? I'm not very familiar with the LL feature or these tests, so it will take me more time to get up to speed.

@theodab / @tykus160 any idea?

[theodab]

The streaming engine tests use a lot of complicated fakes in them. It unfortunately makes those tests difficult to debug. It's possible this is a result of a bug in the shaka.test.FakeSegmentIndex class.

[avelad]

With #7197 the problem is solved, once it is merged, please rebase from the main branch, thanks!

[avelad]

@willdharris can you rebase the PR? Thanks!

[willdharris]

With #7197 the problem is solved, once it is merged, please rebase from the main branch, thanks!

@avelad Thanks for your help with the test!

[avelad]

I'm going to revert this change, sorry!

Due to this change, it is being observed that there are many warning logs of the type:

And it breaks some HLS streams causing the downloaded segments to be duplicated, for example: https://cadena100-cope.flumotion.com/chunks.m3u8