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.

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

refactor: remove duplicate bufferIntersection code #880

Merged
merged 2 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ranges.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export const findSoleUncommonTimeRangesEnd = function(original, update) {
* @param {TimeRanges} bufferB
* @return {TimeRanges} The interesection of `bufferA` with `bufferB`
*/
const bufferIntersection = function(bufferA, bufferB) {
export const bufferIntersection = function(bufferA, bufferB) {
let start = null;
let end = null;
let arity = 0;
Expand Down
12 changes: 10 additions & 2 deletions src/source-updater.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import videojs from 'video.js';
import logger from './util/logger';
import noop from './util/noop';
import { buffered } from './util/buffer';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds like we can delete buffer.js?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woops, removing

import { bufferIntersection } from './ranges.js';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the one in ranges

import {getMimeForCodec} from '@videojs/vhs-utils/dist/codecs.js';

const updating = (type, sourceUpdater) => {
Expand Down Expand Up @@ -322,7 +322,15 @@ export default class SourceUpdater extends videojs.EventTarget {
}

buffered() {
return buffered(this.videoBuffer, this.audioBuffer);
if (this.audioBuffer && !this.videoBuffer) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only unique functionality in that file what using audio or video buffered, if we only had a buffer for one or the other.

return this.audioBuffered();
}

if (this.videoBuffer && !this.audioBuffer) {
return this.videoBuffered();
}

return bufferIntersection(this.audioBuffered(), this.videoBuffered());
}

setDuration(duration, doneFn = noop) {
Expand Down
85 changes: 0 additions & 85 deletions src/util/buffer.js

This file was deleted.

49 changes: 49 additions & 0 deletions test/ranges.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,3 +496,52 @@ QUnit.test('converts time ranges to an array', function(assert) {
'formats ranges correctly'
);
});

QUnit.module('bufferIntersection');

QUnit.test('returns intersection of two buffers', function(assert) {
const a = createTimeRanges([[0, 5], [12, 100]]);
const b = createTimeRanges([[4, 5], [10, 101]]);

assert.ok(
rangesEqual(Ranges.bufferIntersection(a, b), createTimeRanges([[4, 5], [12, 100]])),
'returns intersection'
);
});

QUnit.test('returns empty when no buffers', function(assert) {
assert.ok(
rangesEqual(Ranges.bufferIntersection(null, null), createTimeRanges()),
'returns empty'
);
});

QUnit.test('returns empty when buffers are empty', function(assert) {
const a = createTimeRanges();
const b = createTimeRanges();

assert.ok(
rangesEqual(Ranges.bufferIntersection(a, b), createTimeRanges()),
'returns empty'
);
});

QUnit.test('returns empty when one buffer is empty', function(assert) {
const a = createTimeRanges([[0, 1], [2, 3]]);
const b = createTimeRanges();

assert.ok(
rangesEqual(Ranges.bufferIntersection(a, b), createTimeRanges()),
'returns empty'
);
});

QUnit.test('returns empty when other buffer empty', function(assert) {
const a = createTimeRanges();
const b = createTimeRanges([[0, 1], [2, 3]]);

assert.ok(
rangesEqual(Ranges.bufferIntersection(a, b), createTimeRanges()),
'returns empty'
);
});
36 changes: 36 additions & 0 deletions test/source-updater.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,42 @@ QUnit.test('buffered returns intersection of audio and video buffers', function(
this.sourceUpdater.videoBuffer = origVideoBuffer;
});

QUnit.test('buffered returns audio buffered if no video buffer', function(assert) {
const origAudioBuffer = this.sourceUpdater.audioBuffer;

// mocking the buffered ranges in this test because it's tough to know how much each
// browser will actually buffer
this.sourceUpdater.audioBuffer = {
buffered: videojs.createTimeRanges([[1, 2], [5.5, 5.6], [10.5, 11]])
};

timeRangesEqual(
this.sourceUpdater.buffered(),
this.sourceUpdater.audioBuffered(),
'buffered is audio'
);

this.sourceUpdater.audioBuffer = origAudioBuffer;
});

QUnit.test('buffered returns video buffered if no audio buffer', function(assert) {
const origVideoBuffer = this.sourceUpdater.videoBuffer;

// mocking the buffered ranges in this test because it's tough to know how much each
// browser will actually buffer
this.sourceUpdater.videoBuffer = {
buffered: videojs.createTimeRanges([[1.25, 1.5], [5.1, 6.1], [10.5, 10.9]])
};

timeRangesEqual(
this.sourceUpdater.buffered(),
this.sourceUpdater.videoBuffered(),
'buffered is video'
);

this.sourceUpdater.videoBuffer = origVideoBuffer;
});

QUnit.test('removeAudio removes audio buffer', function(assert) {
const done = assert.async();

Expand Down
92 changes: 0 additions & 92 deletions test/util/buffer.test.js

This file was deleted.