Skip to content

Commit f16ba0f

Browse files
authored
Audio workspace: automatically select newly created interval (#11013)
<!-- Raise an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [Contribution guide](https://docs.cvat.ai/docs/contributing/). --> <!-- Provide a general summary of your changes in the Title above --> ### Motivation and context <!-- Why is this change required? What problem does it solve? If it fixes an open issue, please link to the issue here. Describe your changes in detail, add screenshots. --> Currently, after drawing interval via "Draw" button, double click no longer plays it. It is perceived as a bug and not convenient. The cause is that the audio workspace stays in the "Draw" mode which forbids this and some other actions. This PR implements the following changes. When an audio region is added by any means (draw/extend/record) it automatically: 1. Changes the mode to "Cursor". This effectively resolves the aforementioned bug. Also, makes all similar functions available right away. NOTE: This changes the existing behavior of the audio canvas where one was able to select "Draw" mode and add multiple regions without any additional efforts. The rationale behind this change is: * It doesn't seem common for one to be willing to annotate multiple regions right away as it depends on listening. It's hard to listen to and remember boundaries for multiple intervals in one playback. * It becomes consistent with the standard image canvas behavior which doesn't persist the draw mode after an annotation is added. * It's still very easy to add multiple annotations with shortcuts: **n** shortcut -> Draw Interval -> **n** -> Draw ... It might be useful for tasks where one needs to add multiple annotations with different labels for the same track fragment. 2. Selects the interval as active so the attributes can be filled right away. * This change simply eliminates the need for the explicit natural next step to select it to be able to fill transcription and other attributes which is assumed to be a very common next step. * It doesn't make other scenarios worse where one wouldn't select the interval right away as the next step ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> Tested manually, adjusted e2e tests to account for the change. ### Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. If an item isn't applicable for some reason, then ~~explicitly strikethrough~~ the whole line. If you don't do that, GitHub will show incorrect progress for the pull request. If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [X] I submit my changes into the `develop` branch - [x] I have created a changelog fragment <!-- see top comment in CHANGELOG.md --> - [ ] I have updated the documentation accordingly - [ ] I have added tests to cover my changes - [ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) ### License - [X] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern.
1 parent 03d4bad commit f16ba0f

11 files changed

Lines changed: 44 additions & 19 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
### Changed
2+
3+
- Updated the audio interval creation UX to always switch to "Cursor" mode and
4+
automatically make the newly added interval active
5+
(<https://github.com/cvat-ai/cvat/pull/11013>)

cvat-ui/src/actions/annotation-actions.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,8 +1314,8 @@ export function changeWorkspaceAsync(workspace: Workspace): ThunkAction {
13141314
export function createAnnotationsAsync(
13151315
statesToCreate: (ObjectState | AudioIntervalState)[],
13161316
source: AnnotationSource = AnnotationSource.OTHER,
1317-
): ThunkAction {
1318-
return async (dispatch: ThunkDispatch): Promise<void> => {
1317+
): ThunkAction<Promise<number[]>> {
1318+
return async (dispatch: ThunkDispatch): Promise<number[]> => {
13191319
try {
13201320
const { jobInstance } = receiveAnnotationsParameters();
13211321
const clientIds = await jobInstance.annotations.put(statesToCreate);
@@ -1325,13 +1325,16 @@ export function createAnnotationsAsync(
13251325
const [clientId] = clientIds;
13261326
dispatch(switchSimplifyVisibility(clientId));
13271327
}
1328+
1329+
return clientIds;
13281330
} catch (error) {
13291331
dispatch({
13301332
type: AnnotationActionTypes.CREATE_ANNOTATIONS_FAILED,
13311333
payload: {
13321334
error,
13331335
},
13341336
});
1337+
return [];
13351338
}
13361339
};
13371340
}

cvat-ui/src/actions/audio-actions.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ import {
88
import {
99
AudioIntervalState, FramesMetaData, Job, Source, fetchAndAssembleAudio,
1010
} from 'cvat-core-wrapper';
11+
import { ActiveControl } from 'reducers';
1112
import { clamp } from 'utils/math';
1213
import {
1314
cacheAudioData, removeCachedAudioData,
1415
} from 'audio/utils/audio-data-cache';
16+
import { updateActiveControl } from './annotation-actions';
1517

1618
export enum AudioActionTypes {
1719
SWITCH_AUDIO_PLAY = 'SWITCH_AUDIO_PLAY',
@@ -231,7 +233,10 @@ export function createAudioIntervalAsync(start: number, stop: number, labelID: n
231233
});
232234

233235
const { createAnnotationsAsync } = await import('./annotation-actions');
234-
await dispatch(createAnnotationsAsync([state]));
236+
const [clientID] = await dispatch(createAnnotationsAsync([state]));
237+
238+
dispatch(updateActiveControl(ActiveControl.CURSOR));
239+
dispatch(audioActions.setAudioActiveInterval(clientID));
235240
};
236241
}
237242

cvat-ui/src/audio/components/annotation-page/audio-workspace/hooks/use-audio-recording.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ import {
77
} from 'react';
88
import { useDispatch, useSelector } from 'react-redux';
99

10-
import { audioActions, createAudioIntervalAsync } from 'actions/audio-actions';
11-
import { updateActiveControl } from 'actions/annotation-actions';
10+
import { createAudioIntervalAsync } from 'actions/audio-actions';
1211
import { ActiveControl, CombinedState } from 'reducers';
1312
import { AudioIntervalState, Source } from 'cvat-core-wrapper';
1413
import { shallowEqual, ThunkDispatch } from 'utils/redux';
14+
import { usePrevious } from 'utils/hooks';
1515
import { MIN_INTERVAL_DURATION, MIN_RECORDING_DURATION } from 'audio/utils/waveform-geometry';
1616

1717
import { getAudioRegionColor } from '../audio-region-colors';
@@ -57,7 +57,7 @@ export function useAudioRecording({ playback, regions, ready }: Params): void {
5757
duration, labels, activeLabelID, colorBy, opacity, selectedOpacity,
5858
};
5959
const sessionRef = useRef<RecordingSession | null>(null);
60-
const wasPlayingRef = useRef(playing);
60+
const prevPlaying = usePrevious(playing);
6161

6262
const startSession = useCallback((): void => {
6363
if (sessionRef.current) return;
@@ -105,8 +105,6 @@ export function useAudioRecording({ playback, regions, ready }: Params): void {
105105
sessionRef.current = null;
106106
if (end - session.start < MIN_RECORDING_DURATION) return;
107107

108-
// TODO: is dropping active interval needed here?
109-
dispatch(audioActions.setAudioActiveInterval(null));
110108
dispatch(createAudioIntervalAsync(session.start, end, session.labelID));
111109
}, []);
112110

@@ -131,12 +129,12 @@ export function useAudioRecording({ playback, regions, ready }: Params): void {
131129
}, [activeControl, ready]);
132130

133131
useEffect(() => {
134-
const wasPlaying = wasPlayingRef.current;
135-
wasPlayingRef.current = playing;
136-
if (wasPlaying && !playing && activeControl === ActiveControl.AUDIO_REGION_RECORD) {
137-
dispatch(updateActiveControl(ActiveControl.CURSOR));
132+
if (activeControl !== ActiveControl.AUDIO_REGION_RECORD) return;
133+
134+
if (prevPlaying && !playing) {
135+
finishSession();
138136
}
139-
}, [activeControl, playing]);
137+
}, [activeControl, playing, prevPlaying]);
140138

141139
// update preview region along with playback time updates
142140
useEffect(() => subscribeTimeUpdates(updateSession), []);

tests/cypress/e2e/audio_workspace/case_audio_05_create_via_button.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ context('Audio annotation. Create region via toolbar button.', () => {
1919
cy.get('.cvat-audio-region-item').should('have.length', 0);
2020
cy.audioCreateRegionViaButton(firstLabelName, 100, 250);
2121
cy.get('.cvat-audio-region-item', { timeout: 5000 }).should('have.length', 1);
22-
cy.get('.cvat-audio-region-item').first().should('contain.text', firstLabelName);
22+
cy.get('.cvat-audio-region-item').first()
23+
.should('contain.text', firstLabelName)
24+
.and('have.class', 'cvat-audio-region-item-active');
2325
});
2426
});
2527
});

tests/cypress/e2e/audio_workspace/case_audio_06_create_via_hotkey.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ context('Audio annotation. Create region via hotkey.', () => {
1919
cy.get('.cvat-audio-region-item').should('have.length', 0);
2020
cy.audioCreateRegionViaHotkey(80, 220);
2121
cy.get('.cvat-audio-region-item', { timeout: 5000 }).should('have.length', 1);
22+
cy.get('.cvat-cursor-control').should('have.class', 'cvat-active-canvas-control');
23+
cy.get('.cvat-audio-region-item').first().should('have.class', 'cvat-audio-region-item-active');
2224
});
2325
});
2426
});

tests/cypress/e2e/audio_workspace/case_audio_13_select_region_canvas_click.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@ context('Audio annotation. Click region on canvas activates it.', () => {
1313
cy.prepareUserSession();
1414
cy.openAudioJob(taskName);
1515
cy.audioCreateRegionViaButton(firstLabelName, 100, 250);
16+
cy.audioCreateRegionViaButton(firstLabelName, 300, 450);
1617
});
1718

1819
describe(`Testing case "${caseId}"`, () => {
1920
it('Selecting via list updates the active class on the item', () => {
21+
cy.get('.cvat-audio-region-item').eq(1).should('have.class', 'cvat-audio-region-item-active');
2022
cy.clickRegionOnWaveform((100 + 250) / 2);
2123
cy.get('.cvat-audio-region-item').first().should('have.class', 'cvat-audio-region-item-active');
24+
cy.get('.cvat-audio-region-item').eq(1).should('not.have.class', 'cvat-audio-region-item-active');
2225
cy.get('.cvat-audio-region-details').should('be.visible');
2326
});
2427
});

tests/cypress/e2e/audio_workspace/case_audio_25_extend_via_button.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ context('Audio annotation. Extend region via toolbar button.', () => {
4444
cy.audioExtendViaButton(firstLabelName);
4545

4646
cy.get('.cvat-audio-region-item', { timeout: 5000 }).should('have.length', 3);
47+
cy.get('.cvat-cursor-control').should('have.class', 'cvat-active-canvas-control');
48+
cy.get('.cvat-audio-region-item').last().should('have.class', 'cvat-audio-region-item-active');
4749
getRegionRects().then(([left, extended, right]) => {
4850
expect(extended.left).to.be.closeTo(left.right, REGION_POSITION_TOLERANCE_PX);
4951
expect(extended.right).to.be.lessThan(right.left);
@@ -71,7 +73,10 @@ context('Audio annotation. Extend region via toolbar button.', () => {
7173

7274
cy.audioExtendViaButton(firstLabelName);
7375
cy.get('.cvat-audio-region-item', { timeout: 5000 }).should('have.length', 1);
74-
cy.get('.cvat-audio-region-item').first().should('contain.text', firstLabelName);
76+
cy.get('.cvat-cursor-control').should('have.class', 'cvat-active-canvas-control');
77+
cy.get('.cvat-audio-region-item').first()
78+
.should('contain.text', firstLabelName)
79+
.and('have.class', 'cvat-audio-region-item-active');
7580
cy.get('@pausedCursorPosition').then((pausedCursorPosition) => {
7681
getWaveformWrapper().then(($wrapper) => {
7782
const wrapperLeft = $wrapper[0].getBoundingClientRect().left;

tests/cypress/e2e/audio_workspace/case_audio_26_extend_via_hotkey.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ context('Audio annotation. Extend region via Shift+E hotkey.', () => {
2020
const initial = $body.find('.cvat-audio-region-item').length;
2121
cy.audioCreateRegionViaButton(firstLabelName, 100, 250);
2222
cy.get('.cvat-audio-region-item').should('have.length', initial + 1);
23-
cy.get('body').type('{esc}');
2423
cy.get('.cvat-cursor-control').should('have.class', 'cvat-active-canvas-control');
2524
cy.get('.cvat-audio-waveform-wrapper').first().then(($el) => {
2625
const rect = $el[0].getBoundingClientRect();
@@ -33,7 +32,10 @@ context('Audio annotation. Extend region via Shift+E hotkey.', () => {
3332
cy.get('.cvat-audio-interval-region-popover-content').should('not.be.visible');
3433
cy.get('.cvat-audio-region-item', { timeout: 5000 })
3534
.should('have.length', initial + 2);
36-
cy.get('.cvat-audio-region-item').last().should('contain.text', firstLabelName);
35+
cy.get('.cvat-cursor-control').should('have.class', 'cvat-active-canvas-control');
36+
cy.get('.cvat-audio-region-item').last()
37+
.should('contain.text', firstLabelName)
38+
.and('have.class', 'cvat-audio-region-item-active');
3739
});
3840
});
3941
});

tests/cypress/e2e/audio_workspace/case_audio_30_recording_lifecycle.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ context('Audio annotation. Recording lifecycle.', () => {
3030
cy.get('.cvat-player-pause-button').click();
3131

3232
cy.get('.cvat-cursor-control').should('have.class', 'cvat-active-canvas-control');
33-
cy.get('.cvat-audio-region-item').should('have.length', 1);
33+
cy.get('.cvat-audio-region-item').should('have.length', 1)
34+
.and('have.class', 'cvat-audio-region-item-active');
3435
});
3536

3637
it('Cancels a recording on Escape without creating an interval', () => {

0 commit comments

Comments
 (0)