video-dev / hls.js

HLS.js is a JavaScript library that plays HLS in browsers with support for MSE.
https://hlsjs.video-dev.org/demo
Other
14.8k stars 2.57k forks source link

Call changeType issue #4307

Closed albertdaurell closed 2 years ago

albertdaurell commented 3 years ago

What version of Hls.js are you using?

v1.0.10

What browser (including version) are you using?

Version 92.0.4515.107 (Official Build) (x86_64)

What OS (including version) are you using?

MacOS

Test stream

https://storage.googleapis.com/shaka-demo-assets/angel-one-hls/hls.m3u8

Configuration

{ }

Additional player setup steps

-

Checklist

Steps to reproduce

  1. Open any of the demo hls.js demo urls from a browser with SourceBuffer changeType method defined

  2. From console you can log the issue adding this javascript:

    SourceBuffer.prototype._changeType = SourceBuffer.prototype._changeType || SourceBuffer.prototype.changeType;
    SourceBuffer.prototype.changeType = function (type) {
      console.log('SourceBuffer changeType', type);
      return this._changeType(type);
    };
  3. Now load the sample url https://dev-stpeter-pmd.akamai.cdn.rakuten.tv/g/player/samples/hls_codec_switch/master.m3u8

  4. Check logs from console. There is only SourceBuffer changeType call manifest contains 2 codec switches

Expected behaviour

Two changeType calls should be executed

What actually happened?

Only 1 changeType call is executed

Console output

[log] > [stream-controller]: Init audio buffer, container:audio/mp4, codecs[selected/level/parsed]=[//mp4a.40.5]
[log] > [stream-controller]: Init video buffer, container:video/mp4, codecs[level/parsed]=[/avc1.4d4029]
[log] > [buffer-controller]: changing audio sourceBuffer type to audio/mp4;codecs=mp4a.40.5
# The only one callback to SourceBuffer changeType >>>
VM78:3 SourceBuffer changeType audio/mp4;codecs=mp4a.40.5
[log] > [audio-stream-controller]: InitPTS for cc: 1 found from main: 252032
[log] > [stream-controller]: PARSING->PARSED
[log] > [stream-controller]: Buffered main sn: 1 of level 0 [0.040,7.240]

Chrome media internals output

No response

cjpillsbury commented 3 years ago

Hey @albertdaurell generally speaking, hls.js isn't graceful when switching the actual sources and you'd instead do an unload/teardown when switching streams. From what I can tell, changeType is being used for switching codecs is necessary based on the different media playlists defined in a primary playlist. Given the nonstandard example provided, can you give some more context on what problem(s) you're trying to solve?

albertdaurell commented 3 years ago

Hey @cjpillsbury, thank you very much for checking the issue.

Sorry, about the nonstandard example, I'm not an hls expert šŸ˜…, I did my best trying to give a simplified example of the source code issue.

The main problem we can see with this nonstandard given example is that changeType is not called when it should:

  1. Sometimes it's called although codec did not changed ( not a big issue but... not necessary )
  2. Sometimes it's not called but codec changed ( necessary )

This bad behavior was detected in some Vestel devices from where the video element gets stalled because changeType was not called.

From the sample provided playlist we can see this:

this.tracks stored codec info Playing Codec Next Codec Current behavior Expected behavior Result
A A B changeType is called changeType should be called and this.tracks should be updated āœ… changeType is called
āŒ this.tracks is not updated to B
āŒ A B B changeType is called changeType should NOT be called āš ļø Extra/unnecessary call because B is compared with A NOT with B
āŒ A B A changeType is NOT called changeType should be called and this.tracks should be updated āŒ changeType not called because A is compared with A not with B
āŒ this.tracks is not updated to A

All this bad behavior is caused because when buffer-controller onBufferCodecs method is called, and there was a codec switching, this.tracks[...] is not updated with the current codecs.

And this bad behavior is what I'm trying to fix from https://github.com/video-dev/hls.js/pull/4308, basically adding this lines, in order to update the current codec info:

https://github.com/video-dev/hls.js/pull/4308/files#diff-c4df8184b8ec1cc3d6647f954f28fc69f8d7fe0d54d005ac37adf75b7ac5139bR262-R269

dylanjha commented 3 years ago

Taking a look at this, @albertdaurell interesting find, thanks for the PR that is helpful.

First off, can we take a couple steps back and explain at a high level what you're going after? I think that would help us understand what's going on.

Under the hood, I believe Hls.js uses changeType in order to handle switching between renditions that are using different codecs.

āÆ curl "https://dev-stpeter-pmd.akamai.cdn.rakuten.tv/g/player/samples/hls_codec_switch/master.m3u8"
#EXTM3U
#EXT-X-VERSION:7
#EXT-X-TARGETDURATION:4
#EXT-X-PLAYLIST-TYPE:VOD
#EXT-X-MEDIA-SEQUENCE:0
#EXTINF:3.2,
1.ts
#EXT-X-DISCONTINUITY
#EXTINF:4.0,
2.ts
#EXT-X-DISCONTINUITY
#EXTINF:2.76,
3.ts
#EXT-X-ENDLIST

I see your example here is actually not a primary (aka "master") playlist. This appears to be a media playlist that doesn't specify what the codec is. https://dev-stpeter-pmd.akamai.cdn.rakuten.tv/g/player/samples/hls_codec_switch/master.m3u8

Hls.js can handle media playlists fine, so no problem with that. But since changeType is used by Hls.js to handle switching between renditions that have different codecs, that might be the wrong tree to bark up because this example doesn't have multiple renditions.

The last part, which was mentioned is that if you're going to swap out the src= of the media, Hls.js isn't going to handle that gracefully on the fly. You'd definitely want to teardown and create a new hls instance when you're changing the source. For that reason it would be helpful to step back and understand what you're trying to accomplish. Is swapping out the src= on the fly actually what you're trying to do? Or was that simply your way of re-producing the issue that you're running into?

If you are trying to do something related to the way changeType is intended to be used by Hls.js under the hood then an example primary playlist that has multiple renditions with different codecs would be helpful.

albertdaurell commented 3 years ago

Hi @dylanjha and @cjpillsbury, thank you very much for your patience. Maybe I did not explained ok the main problem. šŸ˜…

This dummy example playlist was extracted from a SSAI live stream and it's intended to be an example of what sometimes can happen in the same rendition ( codec can have a slight change because, for example, an external advertisement was "inserted" )

Hls.js is able to detect this codec change when BUFFER_CODECS event is triggered but it's not calling changeType although MIME type codec changed. This problem is because new codec info is not stored when this BUFFER_CODECS event is received.

Maybe also @robwalch can check this issue, since he is the one that introduced the changeType call in this pull https://github.com/video-dev/hls.js/pull/3358

jlopex commented 2 years ago

Hello @dylanjha

I just wanted to give more context on the request that @albertdaurell made.

We did identify that several SSAI streams may have slightly different codec properties when ads do get inserted even in the very same rendition. For the case of the audio we've observed slightly different sampling rates or different number of channels (mono/stereo), those changes will happen on the very same rendition, we did also observe that those slight changes may make some decoders fail and raise errors, as they end-up receiving unexpected media.

What @albertdaurell proposes is to extend the current changeType detection features present on HLS.js, and with this HLS.js is able to detect that a codec changed from previous media chunk being processed, then notify the mediaSource to handle this upcoming and "slightly" different codec.

I do agree this is probably a corner case and not affecting a lineal live channel where all chunks are being encoded on a live encoder. But from our experience with play-out (VoD static stitched and also SSAI channels), they do tend to have slightly different codec properties during a play session, further investigating we did notice that applying tweak can be very useful when dealing with stitched or SSAI streams on browsers that do support MSE changeType features.

We do appreciate your consideration for this PR as we think this will not break but will enrich the overall experience of users using HLS.js and dealing with such streams.

albertdaurell commented 2 years ago

Hi @robwalch this issue was merged in 1.2.0 from the related pull https://github.com/video-dev/hls.js/pull/4308

robwalch commented 2 years ago

Hi @albertdaurell,

Can you verify against the release at https://hls-js-bbdf933d-da48-407d-aaf3-68cc4ee058e7.netlify.app/demo/ ?

The sample stream is no longer available or cannot be used because of an invalid cert. Thanks!

albertdaurell commented 2 years ago

Hi @albertdaurell,

Can you verify against the release at https://hls-js-bbdf933d-da48-407d-aaf3-68cc4ee058e7.netlify.app/demo/ ?

The sample stream is no longer available or cannot be used because of an invalid cert. Thanks!

Sorry @robwalch I think you already verified the release, otherwise, just in case I did and fix is working fine. Thank you very much!