Closed dzianis-dashkevich closed 1 year ago
Merging #1426 (1d60c53) into main (04451d4) will increase coverage by
0.05%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## main #1426 +/- ##
==========================================
+ Coverage 85.75% 85.81% +0.05%
==========================================
Files 42 42
Lines 10284 10298 +14
Branches 2374 2377 +3
==========================================
+ Hits 8819 8837 +18
+ Misses 1465 1461 -4
Files Changed | Coverage Δ | |
---|---|---|
src/media-segment-request.js | 96.11% <ø> (+0.61%) |
:arrow_up: |
src/playlist-controller.js | 95.36% <ø> (ø) |
|
src/segment-loader.js | 96.69% <100.00%> (+0.19%) |
:arrow_up: |
src/source-updater.js | 94.24% <100.00%> (+0.05%) |
:arrow_up: |
src/util/vjs-compat.js | 100.00% <100.00%> (ø) |
|
src/videojs-http-streaming.js | 91.02% <100.00%> (+0.01%) |
:arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
@dzianis-dashkevich I tested this PR in relation to #1371 and #1049 and it seems that it can also solve these two issues, allowing https://github.com/videojs/mux.js/pull/430 and https://github.com/videojs/mux.js/pull/437 to be closed. It would be great to have 1 or 2 unit tests.
@dzianis-dashkevich I tested this PR in relation to #1371 and #1049 and it seems that it can also solve these two issues, allowing videojs/mux.js#430 and videojs/mux.js#437 to be closed. It would be great to have 1 or 2 unit tests.
@amtins Thank you for your review! I added unit tests.
Regarding closing PR in mux.js... Let's test these changes against many different types of streams to be confident that everything works fine first
This is great! + 1 once all the tests are passing.
It seems like everything is passing now
Description
Sometimes, we may encounter streams with corrupted PTS/DTS timestamps during discontinuities.
Let's review the following
tsanalyze
dump based on these corrupted segments (This specific segment is the very first segment after discontinuity):As you can see, Audio packets have only PTS, and it is monotonically increasing from
0
to180,480
. Video packets have both PTS and DTS values, and DTS is not monotonically increasing: It goes from8,589,916,592
to165,075
.Let's take a look closer at video packets with timestamp info from this segment using
tsdump
:1st video packet with PTS/DTS info:
PTS is
0x000000000
, which is 0, and DTS is0x1FFFFB9B0
, which is8589916592
(33-bit)2nd video packet with PTS/DTS info:
PTS is
0x000001770
, which is6000
, and DTS is0x1FFFFD120
, which is8589922592
(increased by6000
, still 33-bit)3rd video packet with PTS/DTS info:
PTS is
0x000002EE0
, which is12000
, and DTS is0x1FFFFE890
, which is8589928592
(increased by6000
, still 33-bit)4th video packet with PTS/DTS info:
PTS is
0x000004650
, which is18000
, and DTS is0x000000000
, which is0
(it seems like timestamp rollover because according to ISO/IEC 13818-1, DTS/PTS value should be a 33-bit number, but if we take previous value8589928592
+6000
changes it will be8589934592
, which is1000000000000000000000000000000000
in binary and this is 34-bit value, so it is rolling over to0
)Since mux.js relies heavily on DTS values, we have two problems here: Problem 1: Differences between audio and video packets' reported start time in the same segment from
probeTs
:0
vs8589916592
Problem 2: Wrong timestampOffset for next segment after corrupted one.
Since this is a discontinuity segment, we re-calculate
timestampOffset
s based on the segment's timestamps and apply them to source buffers. The next segment will have the same timeline as the corrupted segment. So We do not re-calculatetimestampOffset
, but since the timestamp value was rolled over, we should apply a different timestampOffset for proper time alignment.Specific Changes proposed
probeTs
because Transmuxer aligns time between audio and video in the timestampRollover stream.timestampOffset
for each segment, regardless of its timeline, so timestampOffset should always be valid.I added a feature flag to be safer, but overall, I consider this change risky, as it may break something.
Requirements Checklist