Bug 1845213 Comment 3 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

### MSE buffered range mismatching with the actual data

For the site like [video.js](https://videojs.com/advanced/?video=big-buck-bunny), we fixed the looping issue before in bug 1829086 (added tests as well) but now there is a new issue causing looping failed again.

As some websites can't support seamless looping (they won't append data in time), we would fallback to the non-seamless looping [here](https://searchfox.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#3174-3179) when both tracks are ended. The problem is the Media Format Reader somehow performed seeking to the beginning successfully (it shouldn't), making [mIsReachingAudioEOS](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/MediaDecoderStateMachine.cpp#1163) become `true` so that we can't fallback to the non-seamless looping.

The reason of seeking can be performed is because the **mismatching of buffered range and buffered data in the track buffer manager**. In the media source demuxer, we only want to perform seeking if [the seeking target is in the buffered range](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#379-393). If not, then we would reject the promise with `waiting for data`.

When loading [this video](https://videojs.com/advanced/?video=big-buck-bunny), and performing a seek to the end. That triggers the frame removal algorithm, which supposes to clean all the buffered range which we have before. I added some logs, then found following situation, 

```
RemoveFrames: Removing [(0.000000, 0.009313), (0.009333, 39.838667)] from bufferedRange [(0.000000, 39.838667)]
RemoveFrames: After removing frames, audio/mp4a-latm data sz=0, highestStartTimestamp= 0, bufferedRange=[(0.009313, 0.009333)], sanitizedBufferedRanges=[(0.009313, 0.009333)]
```
The audio buffer actually didn't contain any data, but our buffered range is still `(0.009313, 0.009333)`, it should be `(0,0)`.  When I debugged the crash for bug 1842496, I was so confused why we couldn't [get the next sample](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#400). But now I understand that is because the buffered range was incorrect, so we won't be able to find any data after seeking.

### Root cause for causing mismatch

After more investigation, I noticed that the buffered range is a continuous range `(0.000000, 39.838667)` but the removing range has two intervals, which seems weird because they should have exact same interval sets. I found that the root cause for causing this mismatch is because we modify the sample's time to a wrong time.

By checking the log of sample appending, I found that the data stored in our buffer was actually wrong
```
Expected : [0,9333], [9333,30666], [30666,52000]...
Actual :  [0,1], [1,9313], [9333,30666], [30666,52000]...
```
`[0,1]` was added from [here](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#2056-2069), but the next sample should be `[1,9333]`, not `[1,9313]`. That `9313` was calculated by [adding the duration of interval](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#1849), but the interval's start and end have different base tick (1000000 vs 48000), which causing the calculation error. Making `mStart` with the same base with the `sample->mTime` fixes the problem, and also didn't cause regression for bug 1222851 (in which we introduce that silent frame).
### MSE buffered range mismatching with the actual data

For the site like [video.js](https://videojs.com/advanced/?video=big-buck-bunny), we fixed the looping issue before in bug 1829086 (added tests as well) but now there is a new issue causing looping failed again.

As some websites can't support seamless looping (they won't append data in time), we would fallback to the non-seamless looping [here](https://searchfox.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#3174-3179) when both tracks are ended. The problem is the Media Format Reader somehow performed seeking to the beginning successfully (it shouldn't), making [mIsReachingAudioEOS](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/MediaDecoderStateMachine.cpp#1163) become `true` so that we can't fallback to the non-seamless looping.

The reason of being able to perform seeking is because the **mismatching of buffered range and buffered data in the track buffer manager**. In the media source demuxer, we only want to perform seeking if [the seeking target is in the buffered range](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#379-393). If not, then we would reject the promise with `waiting for data`.

When loading [this video](https://videojs.com/advanced/?video=big-buck-bunny), and performing a seek to the end. That triggers the frame removal algorithm, which supposes to clean all the buffered range which we have before. I added some logs, then found following situation, 

```
RemoveFrames: Removing [(0.000000, 0.009313), (0.009333, 39.838667)] from bufferedRange [(0.000000, 39.838667)]
RemoveFrames: After removing frames, audio/mp4a-latm data sz=0, highestStartTimestamp= 0, bufferedRange=[(0.009313, 0.009333)], sanitizedBufferedRanges=[(0.009313, 0.009333)]
```
The audio buffer actually didn't contain any data, but our buffered range is still `(0.009313, 0.009333)`, it should be `(0,0)`.  When I debugged the crash for bug 1842496, I was so confused why we couldn't [get the next sample](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#400). But now I understand that is because the buffered range was incorrect, so we won't be able to find any data after seeking.

### Root cause for causing mismatch

After more investigation, I noticed that the buffered range is a continuous range `(0.000000, 39.838667)` but the removing range has two intervals, which seems weird because they should have exact same interval sets. I found that the root cause for causing this mismatch is because we modify the sample's time to a wrong time.

By checking the log of sample appending, I found that the data stored in our buffer was actually wrong
```
Expected : [0,9333], [9333,30666], [30666,52000]...
Actual :  [0,1], [1,9313], [9333,30666], [30666,52000]...
```
`[0,1]` was added from [here](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#2056-2069), but the next sample should be `[1,9333]`, not `[1,9313]`. That `9313` was calculated by [adding the duration of interval](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#1849), but the interval's start and end have different base tick (1000000 vs 48000), which causing the calculation error. Making `mStart` with the same base with the `sample->mTime` fixes the problem, and also didn't cause regression for bug 1222851 (in which we introduce that silent frame).
### MSE buffered range mismatching with the actual data

For the site like [video.js](https://videojs.com/advanced/?video=big-buck-bunny), we fixed the looping issue before in bug 1829086 (added tests as well) but now there is a new issue causing looping failed again.

As some websites can't support seamless looping (they won't append data in time), we would fallback to the non-seamless looping [here](https://searchfox.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#3174-3179) when both tracks are ended. The problem is the Media Format Reader somehow performed seeking to the beginning successfully (it shouldn't), making [mIsReachingAudioEOS](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/MediaDecoderStateMachine.cpp#1163) become `true` so that we can't fallback to the non-seamless looping.

The reason of being able to perform seeking is because the **mismatching of buffered range and buffered data in the track buffer manager**. In the media source demuxer, we only want to perform seeking if [the seeking target is in the buffered range](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#379-393). If not, then we would reject the promise with `waiting for data`.

When loading [this video](https://videojs.com/advanced/?video=big-buck-bunny), and performing a seek to the end. That triggers the frame removal algorithm, which supposes to clean all the buffered range which we had before. I added some logs, then found following situation, 

```
RemoveFrames: Removing [(0.000000, 0.009313), (0.009333, 39.838667)] from bufferedRange [(0.000000, 39.838667)]
RemoveFrames: After removing frames, audio/mp4a-latm data sz=0, highestStartTimestamp= 0, bufferedRange=[(0.009313, 0.009333)], sanitizedBufferedRanges=[(0.009313, 0.009333)]
```
The audio buffer actually didn't contain any data, but our buffered range is still `(0.009313, 0.009333)`, it should be `(0,0)`.  When I debugged the crash for bug 1842496, I was so confused why we couldn't [get the next sample](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#400). But now I understand that is because the buffered range was incorrect, so we won't be able to find any data after seeking.

### Root cause for causing mismatch

After more investigation, I noticed that the buffered range is a continuous range `(0.000000, 39.838667)` but the removing range has two intervals, which seems weird because they should have exact same interval sets. I found that the root cause for causing this mismatch is because we modify the sample's time to a wrong time.

By checking the log of sample appending, I found that the data stored in our buffer was actually wrong
```
Expected : [0,9333], [9333,30666], [30666,52000]...
Actual :  [0,1], [1,9313], [9333,30666], [30666,52000]...
```
`[0,1]` was added from [here](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#2056-2069), but the next sample should be `[1,9333]`, not `[1,9313]`. That `9313` was calculated by [adding the duration of interval](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#1849), but the interval's start and end have different base tick (1000000 vs 48000), which causing the calculation error. Making `mStart` with the same base with the `sample->mTime` fixes the problem, and also didn't cause regression for bug 1222851 (in which we introduce that silent frame).
### MSE buffered range mismatching with the actual data

For the site like [video.js](https://videojs.com/advanced/?video=big-buck-bunny), we fixed the looping issue before in bug 1829086 (added tests as well) but now there is a new issue causing looping failed again.

As some websites can't support seamless looping (they won't append data in time), we would fallback to the non-seamless looping [here](https://searchfox.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#3174-3179) when both tracks are ended. The problem is the Media Format Reader somehow performed seeking to the beginning successfully (it shouldn't), making [mIsReachingAudioEOS](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/MediaDecoderStateMachine.cpp#1163) become `true` so that we can't fallback to the non-seamless looping.

The reason of being able to perform seeking is because the **mismatching of buffered range and buffered data in the track buffer manager**. In the media source demuxer, we only want to perform seeking if [the seeking target is in the buffered range](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#379-393). If not, then we would reject the promise with `waiting for data`.

When loading [this video](https://videojs.com/advanced/?video=big-buck-bunny), and performing a seek to the end. That triggers the frame removal algorithm, which supposes to clean all the buffered range which we had before. I added some logs, then found following situation, 

```
RemoveFrames: Removing [(0.000000, 0.009313), (0.009333, 39.838667)] from bufferedRange [(0.000000, 39.838667)]
RemoveFrames: After removing frames, audio/mp4a-latm data sz=0, highestStartTimestamp= 0, bufferedRange=[(0.009313, 0.009333)], sanitizedBufferedRanges=[(0.009313, 0.009333)]
```
The audio buffer actually didn't contain any data, but our buffered range is still `(0.009313, 0.009333)`, it should be `(0,0)`.  When I debugged the crash for bug 1842496, I was so confused why we couldn't [get the next sample](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#400). But now I understand that is because the buffered range was incorrect, so we won't be able to find any data after seeking.

### Root cause for causing mismatch

After more investigation, I noticed that the buffered range is a continuous range `(0.000000, 39.838667)` but the removing range has two intervals, which seems weird because they should have exact same interval sets. I found that the root cause for causing this mismatch is because we modify the sample's time to a wrong time.

By checking the log of sample appending, I found that the data stored in our buffer was actually wrong
```
Expected : [0,9333], [9333,30666], [30666,52000]...
Actual :  [0,1], [1,9313], [9333,30666], [30666,52000]...
```
`[0,1]` was added from [here](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#2056-2069), but the next sample should be `[1,9333]`, not `[1,9313]`. That `9313` was calculated by [adding the duration of interval](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#1849), but the interval's start and end have different base tick (1000000 vs 48000), which causing the calculation error. 
As the minimum unit of the base 48000 would be 20ms, exactly the difference between the current value and the expected value.

Making `mStart` with the same base with the `sample->mTime` fixes the problem, and also doesn't cause regression for bug 1222851 (in which we introduce that silent frame).
### MSE buffered range mismatching with the actual data

For the site like [video.js](https://videojs.com/advanced/?video=big-buck-bunny), we fixed the looping issue before in bug 1829086 (added tests as well) but now there is a new issue causing looping failed again.

As some websites can't support seamless looping (they won't append data in time), we would fallback to the non-seamless looping [here](https://searchfox.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#3174-3179) when both tracks are ended. The problem is the Media Format Reader somehow performed seeking to the beginning successfully (it shouldn't), making [mIsReachingAudioEOS](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/MediaDecoderStateMachine.cpp#1163) become `true` so that we can't fallback to the non-seamless looping.

The reason of being able to perform seeking is because the **mismatching of buffered range and buffered data in the track buffer manager**. In the media source demuxer, we only want to perform seeking if [the seeking target is in the buffered range](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#379-393). If not, then we would reject the promise with `waiting for data`.

When loading [this video](https://videojs.com/advanced/?video=big-buck-bunny), and performing a seek to the end. That triggers the frame removal algorithm, which supposes to clean all the buffered range which we had before. I added some logs, then found following situation, 

```
RemoveFrames: Removing [(0.000000, 0.009313), (0.009333, 39.838667)] from bufferedRange [(0.000000, 39.838667)]
RemoveFrames: After removing frames, audio/mp4a-latm data sz=0, highestStartTimestamp= 0, bufferedRange=[(0.009313, 0.009333)], sanitizedBufferedRanges=[(0.009313, 0.009333)]
```
The audio buffer actually didn't contain any data, but our buffered range is still `(0.009313, 0.009333)`, it should be `(0,0)`.  When I debugged the crash for bug 1842496, I was so confused why we couldn't [get the next sample](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#400). But now I understand that is because the buffered range was incorrect, so we won't be able to find any data after seeking.

### Root cause for causing mismatch

After more investigation, I noticed that the buffered range is a continuous range `(0.000000, 39.838667)` but the removing range has two intervals, which seems weird because they should have exact same interval sets. I found that the root cause for causing this mismatch is because we modify the sample's time to a wrong time.

By checking the log of sample appending, I found that the data stored in our buffer was actually wrong
```
Expected : [0,9333], [9333,30666], [30666,52000]...
Actual :  [0,1], [1,9313], [9333,30666], [30666,52000]...
```
`[0,1]` was added from [here](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#2056-2069), but the next sample should be `[1,9333]`, not `[1,9313]`. That `9313` was calculated by [adding the duration of interval](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#1849), but the interval's start and end have different base tick (1000000 vs 48000), which causing the calculation error.  As the minimum unit of the base 48000 would be `20ms`, exactly the difference between the current value and the expected value.

Making `mStart` with the same base with the `sample->mTime` fixes the problem, and also doesn't cause regression for bug 1222851 (in which we introduce that silent frame).
### MSE buffered range mismatching with the actual data

For the site like [video.js](https://videojs.com/advanced/?video=big-buck-bunny), we fixed the looping issue before in bug 1829086 (added tests as well) but now there is a new issue causing looping failed again.

As some websites can't support seamless looping (they won't append data in time), we would fallback to the non-seamless looping [here](https://searchfox.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#3174-3179) when both tracks are ended. The problem is the Media Format Reader somehow performed seeking to the beginning successfully (it shouldn't), making [mIsReachingAudioEOS](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/MediaDecoderStateMachine.cpp#1163) become `true` so that we can't fallback to the non-seamless looping.

The reason of being able to perform seeking is because the **mismatching of buffered range and buffered data in the track buffer manager**. In the media source demuxer, we only want to perform seeking if [the seeking target is in the buffered range](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#379-393). If not, then we would reject the promise with `waiting for data`.

When loading [this video](https://videojs.com/advanced/?video=big-buck-bunny), and performing a seek to the end. That triggers the frame removal algorithm, which supposes to clean all the buffered range which we had before. I added some logs, then found following situation, 

```
RemoveFrames: Removing [(0.000000, 0.009313), (0.009333, 39.838667)] from bufferedRange [(0.000000, 39.838667)]
RemoveFrames: After removing frames, audio/mp4a-latm data sz=0, highestStartTimestamp= 0, bufferedRange=[(0.009313, 0.009333)], sanitizedBufferedRanges=[(0.009313, 0.009333)]
```
The audio buffer actually didn't contain any data, but our buffered range is still `(0.009313, 0.009333)`, it should be `(0,0)`.  When I debugged the crash for bug 1842496, I was so confused why we couldn't [get the next sample](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/MediaSourceDemuxer.cpp#400). But now I understand that is because the buffered range was incorrect, so we won't be able to find any data after seeking.

### Root cause for causing mismatch

After more investigation, I noticed that the buffered range is a continuous range `(0.000000, 39.838667)` but the removing range has two intervals, which seems weird because they should have exact same interval sets. I found that the root cause for causing this mismatch is because we modify the sample's time to a wrong time.

By checking the log of sample appending, I found that the data stored in our buffer was actually wrong
```
Expected : [0,9333], [9333,30666], [30666,52000]...
Actual :  [0,1], [1,9313], [9333,30666], [30666,52000]...
```
`[0,1]` was added from [here](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#2056-2069), but the next sample should be `[1,9333]`, not `[1,9313]`. That `9313` was calculated by [adding the duration of interval](https://searchfox.org/mozilla-central/rev/d2a61d9c63beb3ac4b134767e43a7a8c1b91d5cd/dom/media/mediasource/TrackBuffersManager.cpp#1849), but the interval's start and end have different base tick (1000000 vs 48000), which causing the calculation error.  As the minimum unit of the base 48000 would be `20ms`, exactly the difference between the current value and the expected value.

Making `mStart` with the same base with the `sample->mTime` fixes the problem, and also doesn't cause regression for bug 1222851 (in which we introduce that silent frame). But that is just a workaround. The proper fix is to round [inbase](https://searchfox.org/mozilla-central/rev/3c7b40d1d74c26a82486f38b5828c3f3a43e05da/dom/media/TimeUnits.cpp#65) in `TimeUnit::FromSeconds()` to reduce the rounding error.

Eg. Doing ` {448/48,000} - {1/1,000,000} ` in `TimeUnit::FromSeconds()`, `inbase` would be `447.95200`. We should use cast it to `448` to have minimum rounding error, instead of `447`.

Back to Bug 1845213 Comment 3