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

Do not split render items on speaker change #393

Merged
merged 1 commit into from
Oct 13, 2022
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 app/src/core/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,5 +396,5 @@ export interface SourceRenderItem {

source: string;
sourceStart: number;
speaker: string;
speaker: string | null;
}
2 changes: 1 addition & 1 deletion app/src/core/otio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export async function exportOtio(
fs.mkdirSync(path.join(outputPath, 'media'));

const timeline: OtioSegment[] = content.map((x) => {
if (!('speaker' in x)) {
if (!('speaker' in x) || x.speaker === null) {
console.debug(x);
throw Error('not implemented');
}
Expand Down
4 changes: 2 additions & 2 deletions app/src/core/player.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ export class Player {
(selectedItems, content) => {
this.pause();
if (selectedItems.length > 0) {
this.renderItems = renderItems(selectedItems);
this.renderItems = renderItems(selectedItems, false);
} else {
this.renderItems = memoizedDocumentRenderItems(content);
this.renderItems = memoizedDocumentRenderItems(content, false);
}
if (this.playing) this.play();
}
Expand Down
2 changes: 1 addition & 1 deletion app/src/pages/Editor/ExportOptions/Audio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function Audio({
>;
}): JSX.Element {
exportCallbackRef.current = async (document, path, progressCallback) => {
const renderItems = memoizedDocumentRenderItems(document.content);
const renderItems = memoizedDocumentRenderItems(document.content, false);
const sources = document.sources;
await exportAudio(renderItems, sources, path, progressCallback);
};
Expand Down
2 changes: 1 addition & 1 deletion app/src/pages/Editor/ExportOptions/Otio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function Otio({
assertSome(state.editor.present);
const server = state.server.servers[state.server.selectedServer];

const ris = memoizedDocumentRenderItems(document.content);
const ris = memoizedDocumentRenderItems(document.content, true);
const sources = document.sources;
await exportOtio(
documentBaseName,
Expand Down
2 changes: 1 addition & 1 deletion app/src/pages/Editor/ExportOptions/Video.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function Video({
const [limitLineLength, setLimitLineLength] = useState(false);
const [lineLimit, setLineLimit] = useState(60);
exportCallbackRef.current = async (document, path, progressCallback) => {
const ris = memoizedDocumentRenderItems(document.content);
const ris = memoizedDocumentRenderItems(document.content, false);
const sources = document.sources;
const vtt = contentToVtt(document.content, false, false, limitLineLength ? lineLimit : null);
await exportVideo(
Expand Down
56 changes: 45 additions & 11 deletions app/src/state/editor/selectors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ test('selected render items', () => {
},
{ type: 'paragraph_end', absoluteStart: 2, absoluteIndex: 4 },
]);
expect(renderItems(selItems)).toStrictEqual([
expect(renderItems(selItems, true)).toStrictEqual([
{
absoluteStart: 1,
length: 2,
Expand Down Expand Up @@ -706,7 +706,7 @@ test('paragraphs fails if no paragraph start before first word', () => {
});

test('renderItems', () => {
expect(memoizedDocumentRenderItems(testContentLong)).toStrictEqual([
expect(memoizedDocumentRenderItems(testContentLong, true)).toStrictEqual([
{
type: 'media',
absoluteStart: 0,
Expand Down Expand Up @@ -735,7 +735,8 @@ test('render items: source silence', () => {
{ type: 'text', source: 'source-1', sourceStart: 2, length: 1, text: 'One', conf: 1 },
{ type: 'non_text', source: 'source-1', sourceStart: 3, length: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand All @@ -757,7 +758,8 @@ test('render items: same source, not-matching time', () => {
{ type: 'text', source: 'source-1', sourceStart: 2, length: 1, text: 'One', conf: 1 },
{ type: 'text', source: 'source-1', sourceStart: 3.1, length: 1, text: 'Two', conf: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand Down Expand Up @@ -787,7 +789,8 @@ test('render items: same source, not-matching time', () => {
{ type: 'text', source: 'source-1', sourceStart: 2, length: 1, text: 'One', conf: 1 },
{ type: 'text', source: 'source-1', sourceStart: 2.9, length: 1, text: 'Two', conf: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand Down Expand Up @@ -817,7 +820,8 @@ test('render items: same source, matching time', () => {
{ type: 'text', source: 'source-1', sourceStart: 2, length: 1, text: 'One', conf: 1 },
{ type: 'text', source: 'source-1', sourceStart: 3, length: 1, text: 'Two', conf: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand All @@ -837,7 +841,8 @@ test('render items: missing paragraph end', () => {
addUuids([
{ type: 'text', source: 'source-1', sourceStart: 2, length: 1, text: 'One', conf: 1 },
{ type: 'text', source: 'source-1', sourceStart: 3, length: 1, text: 'Two', conf: 1 },
])
]),
true
)
).toThrow(/who is the speaker/i);
});
Expand All @@ -852,7 +857,8 @@ test('render items: same source, matching time, matching speaker-name', () => {
{ type: 'paragraph_start', speaker: 'Speaker One', language: 'test' },
{ type: 'text', source: 'source-1', sourceStart: 3, length: 1, text: 'Two', conf: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand All @@ -876,7 +882,8 @@ test('render items: same source, matching time, mismatching speaker-name', () =>
{ type: 'paragraph_start', speaker: 'Speaker Two', language: 'test' },
{ type: 'text', source: 'source-1', sourceStart: 3, length: 1, text: 'Two', conf: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand All @@ -898,6 +905,31 @@ test('render items: same source, matching time, mismatching speaker-name', () =>
]);
});

test('render items: same source, matching time, mismatching speaker-name, ignoring speaker names', () => {
expect(
memoizedDocumentRenderItems(
addUuids([
{ type: 'paragraph_start', speaker: 'Speaker One', language: 'test' },
{ type: 'text', source: 'source-1', sourceStart: 2, length: 1, text: 'One', conf: 1 },
{ type: 'paragraph_end' },
{ type: 'paragraph_start', speaker: 'Speaker Two', language: 'test' },
{ type: 'text', source: 'source-1', sourceStart: 3, length: 1, text: 'Two', conf: 1 },
{ type: 'paragraph_end' },
]),
false
)
).toStrictEqual([
{
type: 'media',
absoluteStart: 0,
length: 2,
source: 'source-1',
sourceStart: 2,
speaker: null,
},
]);
});

test('render items: two silences', () => {
expect(
memoizedDocumentRenderItems(
Expand All @@ -906,7 +938,8 @@ test('render items: two silences', () => {
{ type: 'artificial_silence', length: 1 },
{ type: 'artificial_silence', length: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand All @@ -925,7 +958,8 @@ test('render items: word after silence', () => {
{ type: 'artificial_silence', length: 1 },
{ type: 'text', source: 'source-1', sourceStart: 3, length: 1, text: 'Two', conf: 1 },
{ type: 'paragraph_end' },
])
]),
true
)
).toStrictEqual([
{
Expand Down
19 changes: 12 additions & 7 deletions app/src/state/editor/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,17 @@ function isSubsequentSourceSegment(
}
}

export const memoizedDocumentRenderItems = memoize((content: V3DocumentItem[]): RenderItem[] => {
const timedContent = memoizedTimedDocumentItems(content);
return renderItems(timedContent);
});
export const memoizedDocumentRenderItems = memoize(
(content: V3DocumentItem[], splitOnSpeakerChange: boolean): RenderItem[] => {
const timedContent = memoizedTimedDocumentItems(content);
return renderItems(timedContent, splitOnSpeakerChange);
}
);

export function renderItems(timedContent: V3TimedDocumentItem[]): RenderItem[] {
export function renderItems(
timedContent: V3TimedDocumentItem[],
splitOnSpeakerChange: boolean
): RenderItem[] {
const items = [];
let current: RenderItem | null = null;
let current_speaker: string | null = null;
Expand All @@ -268,7 +273,7 @@ export function renderItems(timedContent: V3TimedDocumentItem[]): RenderItem[] {
getRenderType(item.type) != current.type ||
!isSubsequentSourceSegment(current, item) ||
!isSameSource(current, item) ||
('speaker' in current && current_speaker != current.speaker)
(splitOnSpeakerChange && 'speaker' in current && current_speaker != current.speaker)
) {
if (current) {
items.push(current);
Expand All @@ -287,7 +292,7 @@ export function renderItems(timedContent: V3TimedDocumentItem[]): RenderItem[] {
length,
sourceStart,
source,
speaker: current_speaker,
speaker: splitOnSpeakerChange ? current_speaker : null,
};
break;
}
Expand Down
2 changes: 1 addition & 1 deletion app/src/state/editor/transcript_correction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function getTranscriptCorrectionState(state: EditorState): string {
}

function isSameSourceAndContinuous(items: V3TimedDocumentItem[]): boolean {
return renderItems(items).length == 1;
return renderItems(items, false).length == 1;
}

export const abortTranscriptCorrection = createActionWithReducer<EditorState>(
Expand Down