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

fix bug: subtitle no longer blink in low latency #1314

Merged
merged 3 commits into from
Nov 14, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,21 @@ describe("HTML Text buffer utils - areNearlyEqual", () => {
it("should return true if input number are equals", () => {
expect(areNearlyEqual(5, 5)).toBe(true);
});
it(
"should return false if input number are not nearly equals with delta parameter",
() => {
expect(areNearlyEqual(5, 5.1, 0.02)).toBe(false);
});
it(
"should return true if input number are nearly equals with delta parameter",
() => {
expect(areNearlyEqual(5, 5.01, 0.02)).toBe(true);
});
it(
"should return true if input number are equals with delta parameter",
() => {
expect(areNearlyEqual(5, 5, 0.02)).toBe(true);
});
});

describe("HTML Text buffer utils - removeCuesInfosBetween", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,23 @@ import {
removeCuesInfosBetween,
} from "./utils";

/**
* first or last IHTMLCue in a group can have a slighlty different start
* or end time than the start or end time of the ICuesGroup due to parsing
* approximation.
* DELTA_CUES_GROUP defines the tolerance level when comparing the start/end
* of a IHTMLCue to the start/end of a ICuesGroup.
* Having this value too high may lead to have unwanted subtitle displayed
* Having this value too low may lead to have subtitles not displayed
*/
const DELTA_CUES_GROUP = 1e-3;

/**
* segment_duration / RELATIVE_DELTA_RATIO = relative_delta
*
* relative_delta is the tolerance to determine if two segements are the same
*/
const RELATIVE_DELTA_RATIO = 5;
peaBerberian marked this conversation as resolved.
Show resolved Hide resolved
/**
* Manage the buffer of the HTMLTextSegmentBuffer.
* Allows to add, remove and recuperate cues at given times.
Expand Down Expand Up @@ -72,6 +89,19 @@ export default class TextTrackCuesStore {
ret.push(cues[j].element);
}
}
// first or last IHTMLCue in a group can have a slighlty different start
// or end time than the start or end time of the ICuesGroup due to parsing
// approximation.
// Add a tolerance of 1ms to fix this issue
if (ret.length === 0 && cues.length > 0) {
for (let j = 0; j < cues.length; j++) {
if (areNearlyEqual(time, cues[j].start, DELTA_CUES_GROUP)
|| areNearlyEqual(time, cues[j].end, DELTA_CUES_GROUP)
) {
ret.push(cues[j].element);
}
}
}
peaBerberian marked this conversation as resolved.
Show resolved Hide resolved
return ret;
}
}
Expand Down Expand Up @@ -163,6 +193,11 @@ export default class TextTrackCuesStore {
insert(cues : IHTMLCue[], start : number, end : number) : void {
const cuesBuffer = this._cuesBuffer;
const cuesInfosToInsert = { start, end, cues };
// it's preferable to have a delta depending on the duration of the segment
// if the delta is one fifth of the length of the segment:
// a segment of [0, 2] is the "same" segment as [0, 2.1]
// but [0, 0.04] is not the "same" segement as [0,04, 0.08]
const relativeDelta = Math.abs(start - end) / RELATIVE_DELTA_RATIO;

/**
* Called when we found the index of the next cue relative to the cue we
Expand All @@ -175,7 +210,7 @@ export default class TextTrackCuesStore {
function onIndexOfNextCueFound(indexOfNextCue : number) : void {
const nextCue = cuesBuffer[indexOfNextCue];
if (nextCue === undefined || // no cue
areNearlyEqual(cuesInfosToInsert.end, nextCue.end)) // samey end
areNearlyEqual(cuesInfosToInsert.end, nextCue.end, relativeDelta)) // samey end
{
// ours: |AAAAA|
// the current one: |BBBBB|
Expand Down Expand Up @@ -210,8 +245,8 @@ export default class TextTrackCuesStore {
for (let cueIdx = 0; cueIdx < cuesBuffer.length; cueIdx++) {
let cuesInfos = cuesBuffer[cueIdx];
if (start < cuesInfos.end) {
if (areNearlyEqual(start, cuesInfos.start)) {
if (areNearlyEqual(end, cuesInfos.end)) {
if (areNearlyEqual(start, cuesInfos.start, relativeDelta)) {
if (areNearlyEqual(end, cuesInfos.end, relativeDelta)) {
// exact same segment
// ours: |AAAAA|
// the current one: |BBBBB|
Expand Down Expand Up @@ -257,7 +292,7 @@ export default class TextTrackCuesStore {
// - add ours before the current one
cuesBuffer.splice(cueIdx, 0, cuesInfosToInsert);
return;
} else if (areNearlyEqual(end, cuesInfos.start)) {
} else if (areNearlyEqual(end, cuesInfos.start, relativeDelta)) {
// our cue goes just before the current one:
// ours: |AAAAAAA|
// the current one: |BBBB|
Expand All @@ -268,7 +303,7 @@ export default class TextTrackCuesStore {
cuesInfos.start = end;
cuesBuffer.splice(cueIdx, 0, cuesInfosToInsert);
return;
} else if (areNearlyEqual(end, cuesInfos.end)) {
} else if (areNearlyEqual(end, cuesInfos.end, relativeDelta)) {
// ours: |AAAAAAA|
// the current one: |BBBB|
// Result: |AAAAAAA|
Expand Down Expand Up @@ -297,7 +332,7 @@ export default class TextTrackCuesStore {
}
// else -> start > cuesInfos.start

if (areNearlyEqual(cuesInfos.end, end)) {
if (areNearlyEqual(cuesInfos.end, end, relativeDelta)) {
// ours: |AAAAAA|
// the current one: |BBBBBBBB|
// Result: |BBAAAAAA|
Expand Down Expand Up @@ -333,6 +368,22 @@ export default class TextTrackCuesStore {
}
}
}

if (cuesBuffer.length) {
const lastCue = cuesBuffer[cuesBuffer.length - 1];
if (areNearlyEqual(lastCue.end, start, relativeDelta)) {
// Match the end of the previous cue to the start of the following one
// if they are close enough. If there is a small gap between two segments
// it can lead to having no subtitles for a short time, this is noticeable when
// two successive segments displays the same text, making it diseappear
// and reappear quickly, which gives the impression of blinking
//
// ours: |AAAAA|
// the current one: |BBBBB|...
// Result: |BBBBBBBAAAAA|
lastCue.end = start;
}
}
// no cues group has the end after our current start.
// These cues should be the last one
cuesBuffer.push(cuesInfosToInsert);
Expand Down
21 changes: 19 additions & 2 deletions src/core/segment_buffers/implementations/text/html/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@ import {
* Setting a value too high might lead to two segments targeting different times
* to be wrongly believed to target the same time. In worst case scenarios, this
* could lead to wanted text tracks being removed.
*
* When comparing 2 segments s1 and s2, you may want to take into account the duration
* of the segments:
* - if s1 is [0, 2] and s2 is [0, 2.1] s1 and s2 can be considered as nearly equal as
* there is a relative difference of: (2.1-2) / 2 = 5%;
* Formula: (end_s1 - end_s2) / duration_s2 = relative_difference
* - if s1 is [0, 0.04] and s2 is [0.04, 0.08] s1 and s2 may not considered as nearly
* equal as there is a relative difference of: (0.04-0.08) / 0.04 = 100%
*
* To compare relatively to the duration of a segment you can provide and additional
* parameter "delta" that remplace MAX_DELTA_BUFFER_TIME.
* If parameter "delta" is higher than MAX_DELTA_BUFFER_TIME, MAX_DELTA_BUFFER_TIME
* is used instead of delta. This ensure that segments are nearly equal when comparing
* relatively AND absolutely.
peaBerberian marked this conversation as resolved.
Show resolved Hide resolved
*
* @type Number
*/
const MAX_DELTA_BUFFER_TIME = 0.2;
Expand All @@ -58,10 +73,12 @@ const MAX_DELTA_BUFFER_TIME = 0.2;
* @see MAX_DELTA_BUFFER_TIME
* @param {Number} a
* @param {Number} b
* @param {Number} delta
* @returns {Boolean}
*/
export function areNearlyEqual(a : number, b : number) : boolean {
return Math.abs(a - b) <= MAX_DELTA_BUFFER_TIME;
export function areNearlyEqual(
a : number, b : number, delta: number = MAX_DELTA_BUFFER_TIME) : boolean {
return Math.abs(a - b) <= Math.min(delta, MAX_DELTA_BUFFER_TIME);
}

/**
Expand Down
Loading