Skip to content

Commit

Permalink
fix(period flattening): fix attribute comparison in the period flatte…
Browse files Browse the repository at this point in the history
…ning logic.

Issue #2716.

Change-Id: I8931dc31d0208ad9cfa25c9d87fcf3cbf550f423
  • Loading branch information
ismena committed Aug 7, 2020
1 parent 1c00b4c commit 4781129
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 15 deletions.
40 changes: 25 additions & 15 deletions lib/util/periods.js
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ shaka.util.PeriodCombiner = class {
output.drmInfos = commonDrmInfos;

// The output is encrypted if any input was encrypted.
output.encrypted = output.encrypted && input.encrypted;
output.encrypted = output.encrypted || input.encrypted;

// Take the max bandwidth, resolution, frame rate, sample rate, and channel
// count.
Expand Down Expand Up @@ -953,7 +953,7 @@ shaka.util.PeriodCombiner = class {
*/
static isAudioStreamBetterMatch_(outputStream, best, candidate) {
const LanguageUtils = shaka.util.LanguageUtils;
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse_;
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse;

// If the output stream was based on the candidate stream, the candidate
// stream should be considered a better match. We can check this by
Expand Down Expand Up @@ -1018,7 +1018,7 @@ shaka.util.PeriodCombiner = class {
// If language-based and role-based features are equivalent, take the audio
// with the closes channel count to the output.
const channelsBetterOrWorse =
shaka.util.PeriodCombiner.compareClosestPreferLower_(
shaka.util.PeriodCombiner.compareClosestPreferLower(
outputStream.channelsCount,
best.channelsCount,
candidate.channelsCount);
Expand All @@ -1030,7 +1030,7 @@ shaka.util.PeriodCombiner = class {

// If channels are equal, take the closest sample rate to the output.
const sampleRateBetterOrWorse =
shaka.util.PeriodCombiner.compareClosestPreferLower_(
shaka.util.PeriodCombiner.compareClosestPreferLower(
outputStream.audioSamplingRate,
best.audioSamplingRate,
candidate.audioSamplingRate);
Expand Down Expand Up @@ -1069,7 +1069,7 @@ shaka.util.PeriodCombiner = class {
* @private
*/
static isVideoStreamBetterMatch_(outputStream, best, candidate) {
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse_;
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse;

// If the output stream was based on the candidate stream, the candidate
// stream should be considered a better match. We can check this by
Expand All @@ -1083,7 +1083,7 @@ shaka.util.PeriodCombiner = class {

// Take the video with the closest resolution to the output.
const resolutionBetterOrWorse =
shaka.util.PeriodCombiner.compareClosestPreferLower_(
shaka.util.PeriodCombiner.compareClosestPreferLower(
outputStream.width * outputStream.height,
best.width * best.height,
candidate.width * candidate.height);
Expand All @@ -1098,7 +1098,7 @@ shaka.util.PeriodCombiner = class {
if (outputStream.frameRate) {
// Take the video with the closest frame rate to the output.
const frameRateBetterOrWorse =
shaka.util.PeriodCombiner.compareClosestPreferLower_(
shaka.util.PeriodCombiner.compareClosestPreferLower(
outputStream.frameRate,
best.frameRate,
candidate.frameRate);
Expand Down Expand Up @@ -1286,11 +1286,18 @@ shaka.util.PeriodCombiner = class {
* @param {number} outputValue
* @param {number} bestValue
* @param {number} candidateValue
* @return {shaka.util.PeriodCombiner.BetterOrWorse_}
* @private
* @return {shaka.util.PeriodCombiner.BetterOrWorse}
*/
static compareClosestPreferLower_(outputValue, bestValue, candidateValue) {
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse_;
static compareClosestPreferLower(outputValue, bestValue, candidateValue) {
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse;

// If one is the exact match for the output value, and the other isn't,
// prefer the one that is the exact match.
if (bestValue == outputValue && outputValue != candidateValue) {
return WORSE;
} else if (candidateValue == outputValue && outputValue != bestValue) {
return BETTER;
}

if (bestValue > outputValue) {
if (candidateValue <= outputValue) {
Expand All @@ -1303,6 +1310,8 @@ shaka.util.PeriodCombiner = class {
// whichever is closer.
if (candidateValue - outputValue < bestValue - outputValue) {
return BETTER;
} else if (candidateValue - outputValue > bestValue - outputValue) {
return WORSE;
}
} else {
// The "best" so far is less than or equal to the output. If the
Expand All @@ -1315,6 +1324,8 @@ shaka.util.PeriodCombiner = class {
// Take whichever is closer.
if (outputValue - candidateValue < outputValue - bestValue) {
return BETTER;
} else if (outputValue - candidateValue > outputValue - bestValue) {
return WORSE;
}
}

Expand All @@ -1325,12 +1336,12 @@ shaka.util.PeriodCombiner = class {
* @param {number} outputValue
* @param {number} bestValue
* @param {number} candidateValue
* @return {shaka.util.PeriodCombiner.BetterOrWorse_}
* @return {shaka.util.PeriodCombiner.BetterOrWorse}
* @private
*/
static compareClosestPreferMinimalAbsDiff_(
outputValue, bestValue, candidateValue) {
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse_;
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse;

const absDiffBest = Math.abs(outputValue - bestValue);
const absDiffCandidate = Math.abs(outputValue - candidateValue);
Expand Down Expand Up @@ -1367,9 +1378,8 @@ shaka.util.PeriodCombiner.Period;

/**
* @enum {number}
* @private
*/
shaka.util.PeriodCombiner.BetterOrWorse_ = {
shaka.util.PeriodCombiner.BetterOrWorse = {
BETTER: 1,
EQUAL: 0,
WORSE: -1,
Expand Down
49 changes: 49 additions & 0 deletions test/util/periods_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,55 @@ describe('PeriodCombiner', () => {
expect(audio2.originalId).toBe('stream2,stream4');
});

describe('compareClosestPreferLower', () => {
const PeriodCombiner = shaka.util.PeriodCombiner;
const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse;

it('Prefers value equal to the output', () => {
let isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 5, /* candidateValue= */ 3);
expect(isCandidateBetter).toBe(WORSE);

// Make sure it works correctly whether it's the candidate or the best
// value that is equel to the output.
isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 3, /* candidateValue= */ 5);
expect(isCandidateBetter).toBe(BETTER);
});

it('Prefers a value lower than the output', () => {
let isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 3, /* candidateValue= */ 6);
expect(isCandidateBetter).toBe(WORSE);

isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 7, /* candidateValue= */ 2);
expect(isCandidateBetter).toBe(BETTER);
});

it('If both values are lower than the output,' +
' prefer the one that\'s closer', () => {
let isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 4, /* candidateValue= */ 3);
expect(isCandidateBetter).toBe(WORSE);

isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 2, /* candidateValue= */ 3);
expect(isCandidateBetter).toBe(BETTER);
});

it('If both values are greater than the output,' +
' prefer the one that\'s closer', () => {
let isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 6, /* candidateValue= */ 7);
expect(isCandidateBetter).toBe(WORSE);

isCandidateBetter = PeriodCombiner.compareClosestPreferLower(
/* output= */ 5, /* bestValue= */ 9, /* candidateValue= */ 8);
expect(isCandidateBetter).toBe(BETTER);
});
});

/** @type {number} */
let nextId = 0;

Expand Down

0 comments on commit 4781129

Please sign in to comment.