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.

Sign up for GitHub

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

Jump to bottom

Bugfix: Store track info when buffer codec switched #4308

Merged
merged 6 commits into from
Jan 18, 2022
Merged

Bugfix: Store track info when buffer codec switched #4308

merged 6 commits into from
Jan 18, 2022

Conversation

albertdaurell
Copy link
Contributor

@albertdaurell albertdaurell commented Sep 3, 2021 •
edited

This PR will...

Ensure changeType is called in the correct scenarios, for example, if codec is the same than the first detected codec from createSourceBuffers.

The problem is that:

  • When codec is created from createSourceBuffers codecs info is stored.
  • But when codec switches from onBufferCodecs the codec info is not overridden/stored.

Resuming, this scenario will imply some errors, a use case example:

  1. createSourceBuffers is called with audio/mp4 mp4a.40.2
  2. onBufferCodecs called with audio/mp4 mp4a.40.5
    OK changeType called because it differs from mp4a.40.2
  3. onBufferCodecs called with audio/mp4 mp4a.40.5
    WARNING unnecessary changeType called because it differs from mp4a.40.2 but codec is same than previous mp4a.40.5
  4. onBufferCodecs called with audio/mp4 mp4a.40.2
    ERROR changeType is NOT called because it's compared with original mp4a.40.2 but it really differs from previous codec mp4a.40.5

Why is this Pull Request needed?

If changeType is not called we found that some Vestel devices are having issues while playing some streams.
Also we'll remove some unnecessary calls to changeType when codec did not really changed.

Are there any points in the code the reviewer needs to double check?

If you want we can add add the hls media url in the tests/test-streams.js commented in the issue?

Resolves issues:

Fix issue #4307

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@albertdaurell albertdaurell force-pushed the bugfix/ensure-changetype-called-when-required branch 2 times, most recently from 424f4f6 to a66184e Compare September 3, 2021 13:02
@albertdaurell albertdaurell force-pushed the bugfix/ensure-changetype-called-when-required branch from a66184e to da99b8d Compare September 3, 2021 13:03
@albertdaurell albertdaurell changed the title Store track info when buffer codec switched Bugfix: Store track info when buffer codec switched Sep 9, 2021
@albertdaurell albertdaurell mentioned this pull request Sep 27, 2021
Closed
5 tasks
@albertdaurell
Copy link
Contributor Author

albertdaurell commented Dec 24, 2021 •
edited

@robwalch Any feedback about this? We already created the issue #4307 but it's still under discussion. This is an improvement/protection it can help to improve buffer/stall issues, specially on embedded devices ( e.g. TVs ).

Thank you very much

@robwalch robwalch added this to the 1.2.0 milestone Dec 24, 2021
@robwalch robwalch merged commit 9781184 into video-dev:master Jan 18, 2022
littlespex pushed a commit to cbsinteractive/hls.js that referenced this pull request Dec 9, 2022
@JulianDomingo
fix: Fix A/V sync in unaligned HLS VOD streams (video-dev#4528)
8c198df 
Fix is based on suggestions from @joeyparrish in
shaka-project/shaka-player#4308 (comment).

During automatic adaptations, Shaka will now reset the timestamp offset
to ensure the newly active track is properly aligned in the
presentation.

Verified by disabling ABR and manually triggering variant switches
(`shaka.Player.selectVariantTrack()`) between seemingly problematic
combinations (e.g., `400k` bps => `6000k` bps stream). Behavior was
compared against production.

Since `adaptation` events are only triggered by ABR logic (and ABR was
disabled for manual testing), `selectVariantTrack()` logic was
temporarily changed from:

`this.switchVariant_(variant, /* fromAdaptation= */ false, clearBuffer,
safeMargin);`
=>
`this.switchVariant_(variant, /* fromAdaptation= */ true, clearBuffer,
safeMargin);`

to ensure the fixes proposed in this PR were taken into effect and being
used during manual testing. Tested content is from the reported bug,
located here:
shaka-project/shaka-player#4308 (comment)

Closes video-dev#4308
littlespex pushed a commit to cbsinteractive/hls.js that referenced this pull request Dec 9, 2022
@theodab
feat(HLS): Lazy-load HLS media playlists (video-dev#4511)
b2f279d 
This changes the HLS parser so that the media playlists are only downloaded when the createSegmentIndex function for the associated stream is called.
Because there is some important information about HLS streams that is stored inside the media playlist, this also changes the player to call createSegmentIndex on the initial variant earlier in the load process, to make sure that information is available in time.
As of this change, we will now require HLS streams to be aligned (see video-dev#4308) for livestreams. VOD content can still
be unaligned.

Closes video-dev#1936
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robwalch robwalch robwalch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@albertdaurell @robwalch