Address video progress review feedback
This commit is contained in:
committed by
Samuel Newman
parent
f5ca5b14cc
commit
4cbd643964
@@ -25,7 +25,6 @@ import {Circle as ProgressCircle} from 'react-native-progress'
|
|||||||
import Animated, {
|
import Animated, {
|
||||||
type AnimatedRef,
|
type AnimatedRef,
|
||||||
type AnimatedStyle,
|
type AnimatedStyle,
|
||||||
Easing,
|
|
||||||
FadeIn,
|
FadeIn,
|
||||||
FadeOut,
|
FadeOut,
|
||||||
interpolateColor,
|
interpolateColor,
|
||||||
@@ -37,7 +36,6 @@ import Animated, {
|
|||||||
useAnimatedStyle,
|
useAnimatedStyle,
|
||||||
useDerivedValue,
|
useDerivedValue,
|
||||||
useSharedValue,
|
useSharedValue,
|
||||||
withRepeat,
|
|
||||||
withTiming,
|
withTiming,
|
||||||
ZoomIn,
|
ZoomIn,
|
||||||
ZoomOut,
|
ZoomOut,
|
||||||
@@ -180,6 +178,7 @@ import {
|
|||||||
processVideo,
|
processVideo,
|
||||||
type VideoState,
|
type VideoState,
|
||||||
} from './state/video'
|
} from './state/video'
|
||||||
|
import {videoProgressWithinPhase} from './state/videoProgress'
|
||||||
import {type TextInputRef} from './text-input/TextInput.types'
|
import {type TextInputRef} from './text-input/TextInput.types'
|
||||||
import {getVideoMetadata} from './videos/metadata'
|
import {getVideoMetadata} from './videos/metadata'
|
||||||
import {clearThumbnailCache} from './videos/VideoTranscodeBackdrop'
|
import {clearThumbnailCache} from './videos/VideoTranscodeBackdrop'
|
||||||
@@ -1996,7 +1995,10 @@ function ComposerEmbeds({
|
|||||||
(video.status === 'compressing' ? (
|
(video.status === 'compressing' ? (
|
||||||
<VideoTranscodeProgress
|
<VideoTranscodeProgress
|
||||||
asset={video.asset}
|
asset={video.asset}
|
||||||
progress={video.progress}
|
progress={videoProgressWithinPhase(
|
||||||
|
'compressing',
|
||||||
|
video.progress,
|
||||||
|
)}
|
||||||
clear={clearVideo}
|
clear={clearVideo}
|
||||||
/>
|
/>
|
||||||
) : video.video ? (
|
) : video.video ? (
|
||||||
@@ -2672,31 +2674,8 @@ function VideoUploadToolbar({state}: {state: VideoState}) {
|
|||||||
const t = useTheme()
|
const t = useTheme()
|
||||||
const {t: l} = useLingui()
|
const {t: l} = useLingui()
|
||||||
const progress = state.progress
|
const progress = state.progress
|
||||||
const processingProgress =
|
|
||||||
state.status === 'processing' ? state.jobStatus?.progress : undefined
|
|
||||||
const shouldRotate =
|
|
||||||
state.status === 'processing' && processingProgress === undefined
|
|
||||||
let wheelProgress = progress
|
let wheelProgress = progress
|
||||||
|
|
||||||
const rotate = useDerivedValue(() => {
|
|
||||||
if (shouldRotate) {
|
|
||||||
return withRepeat(
|
|
||||||
withTiming(360, {
|
|
||||||
duration: 2500,
|
|
||||||
easing: Easing.out(Easing.cubic),
|
|
||||||
}),
|
|
||||||
-1,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
return 0
|
|
||||||
})
|
|
||||||
|
|
||||||
const animatedStyle = useAnimatedStyle(() => {
|
|
||||||
return {
|
|
||||||
transform: [{rotateZ: `${rotate.get()}deg`}],
|
|
||||||
}
|
|
||||||
})
|
|
||||||
|
|
||||||
let text = ''
|
let text = ''
|
||||||
|
|
||||||
const isGif = state.video?.mimeType === 'image/gif'
|
const isGif = state.video?.mimeType === 'image/gif'
|
||||||
@@ -2738,19 +2717,17 @@ function VideoUploadToolbar({state}: {state: VideoState}) {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<ToolbarWrapper style={[a.flex_row, a.align_center, {paddingVertical: 5}]}>
|
<ToolbarWrapper style={[a.flex_row, a.align_center, {paddingVertical: 5}]}>
|
||||||
<Animated.View style={[animatedStyle]}>
|
<ProgressCircle
|
||||||
<ProgressCircle
|
size={30}
|
||||||
size={30}
|
borderWidth={1}
|
||||||
borderWidth={1}
|
borderColor={t.atoms.border_contrast_low.borderColor}
|
||||||
borderColor={t.atoms.border_contrast_low.borderColor}
|
color={
|
||||||
color={
|
state.status === 'error'
|
||||||
state.status === 'error'
|
? t.palette.negative_500
|
||||||
? t.palette.negative_500
|
: t.palette.primary_500
|
||||||
: t.palette.primary_500
|
}
|
||||||
}
|
progress={wheelProgress}
|
||||||
progress={wheelProgress}
|
/>
|
||||||
/>
|
|
||||||
</Animated.View>
|
|
||||||
<Text style={[a.font_semi_bold, a.ml_sm]}>{text}</Text>
|
<Text style={[a.font_semi_bold, a.ml_sm]}>{text}</Text>
|
||||||
</ToolbarWrapper>
|
</ToolbarWrapper>
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ export type VideoAction =
|
|||||||
| {
|
| {
|
||||||
type: 'compressing_to_uploading'
|
type: 'compressing_to_uploading'
|
||||||
video: CompressedVideo
|
video: CompressedVideo
|
||||||
|
compressionSkipped: boolean
|
||||||
signal: AbortSignal
|
signal: AbortSignal
|
||||||
}
|
}
|
||||||
| {
|
| {
|
||||||
@@ -103,6 +104,7 @@ type CompressingState = {
|
|||||||
type UploadingState = {
|
type UploadingState = {
|
||||||
status: 'uploading'
|
status: 'uploading'
|
||||||
progress: number
|
progress: number
|
||||||
|
compressionSkipped: boolean
|
||||||
abortController: AbortController
|
abortController: AbortController
|
||||||
asset: ImagePickerAsset
|
asset: ImagePickerAsset
|
||||||
video: CompressedVideo
|
video: CompressedVideo
|
||||||
@@ -186,13 +188,13 @@ export function videoReducer(
|
|||||||
}
|
}
|
||||||
} else if (action.type === 'update_progress') {
|
} else if (action.type === 'update_progress') {
|
||||||
if (state.status === 'compressing' || state.status === 'uploading') {
|
if (state.status === 'compressing' || state.status === 'uploading') {
|
||||||
|
const phase =
|
||||||
|
state.status === 'uploading' && state.compressionSkipped
|
||||||
|
? 'uploadingWithoutCompression'
|
||||||
|
: state.status
|
||||||
return {
|
return {
|
||||||
...state,
|
...state,
|
||||||
progress: advanceVideoProgress(
|
progress: advanceVideoProgress(state.progress, phase, action.progress),
|
||||||
state.progress,
|
|
||||||
state.status,
|
|
||||||
action.progress,
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if (action.type === 'update_alt_text') {
|
} else if (action.type === 'update_alt_text') {
|
||||||
@@ -209,7 +211,13 @@ export function videoReducer(
|
|||||||
if (state.status === 'compressing') {
|
if (state.status === 'compressing') {
|
||||||
return {
|
return {
|
||||||
status: 'uploading',
|
status: 'uploading',
|
||||||
progress: videoProgressForPhase('uploading', 0),
|
progress: videoProgressForPhase(
|
||||||
|
action.compressionSkipped
|
||||||
|
? 'uploadingWithoutCompression'
|
||||||
|
: 'uploading',
|
||||||
|
0,
|
||||||
|
),
|
||||||
|
compressionSkipped: action.compressionSkipped,
|
||||||
abortController: state.abortController,
|
abortController: state.abortController,
|
||||||
asset: state.asset,
|
asset: state.asset,
|
||||||
video: action.video,
|
video: action.video,
|
||||||
@@ -323,6 +331,7 @@ export async function processVideo(
|
|||||||
dispatch({
|
dispatch({
|
||||||
type: 'compressing_to_uploading',
|
type: 'compressing_to_uploading',
|
||||||
video,
|
video,
|
||||||
|
compressionSkipped: video.passthroughReason !== undefined,
|
||||||
signal,
|
signal,
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,8 @@
|
|||||||
import {advanceVideoProgress, videoProgressForPhase} from './videoProgress'
|
import {
|
||||||
|
advanceVideoProgress,
|
||||||
|
videoProgressForPhase,
|
||||||
|
videoProgressWithinPhase,
|
||||||
|
} from './videoProgress'
|
||||||
|
|
||||||
describe('videoProgressForPhase', () => {
|
describe('videoProgressForPhase', () => {
|
||||||
it('maps each phase onto one continuous timeline', () => {
|
it('maps each phase onto one continuous timeline', () => {
|
||||||
@@ -6,6 +10,8 @@ describe('videoProgressForPhase', () => {
|
|||||||
expect(videoProgressForPhase('compressing', 1)).toBe(0.3)
|
expect(videoProgressForPhase('compressing', 1)).toBe(0.3)
|
||||||
expect(videoProgressForPhase('uploading', 0)).toBe(0.3)
|
expect(videoProgressForPhase('uploading', 0)).toBe(0.3)
|
||||||
expect(videoProgressForPhase('uploading', 1)).toBe(0.5)
|
expect(videoProgressForPhase('uploading', 1)).toBe(0.5)
|
||||||
|
expect(videoProgressForPhase('uploadingWithoutCompression', 0)).toBe(0)
|
||||||
|
expect(videoProgressForPhase('uploadingWithoutCompression', 1)).toBe(0.5)
|
||||||
expect(videoProgressForPhase('processing', 0)).toBe(0.5)
|
expect(videoProgressForPhase('processing', 0)).toBe(0.5)
|
||||||
expect(videoProgressForPhase('processing', 0.5)).toBe(0.75)
|
expect(videoProgressForPhase('processing', 0.5)).toBe(0.75)
|
||||||
expect(videoProgressForPhase('processing', 1)).toBe(1)
|
expect(videoProgressForPhase('processing', 1)).toBe(1)
|
||||||
@@ -17,6 +23,19 @@ describe('videoProgressForPhase', () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('videoProgressWithinPhase', () => {
|
||||||
|
it('maps global progress back to phase-local progress', () => {
|
||||||
|
expect(videoProgressWithinPhase('compressing', 0)).toBe(0)
|
||||||
|
expect(videoProgressWithinPhase('compressing', 0.15)).toBe(0.5)
|
||||||
|
expect(videoProgressWithinPhase('compressing', 0.3)).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('clamps progress outside the phase', () => {
|
||||||
|
expect(videoProgressWithinPhase('compressing', -1)).toBe(0)
|
||||||
|
expect(videoProgressWithinPhase('compressing', 1)).toBe(1)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe('advanceVideoProgress', () => {
|
describe('advanceVideoProgress', () => {
|
||||||
it('does not move backwards when a transport retries or falls back', () => {
|
it('does not move backwards when a transport retries or falls back', () => {
|
||||||
const progressed = advanceVideoProgress(0.5, 'uploading', 0.8)
|
const progressed = advanceVideoProgress(0.5, 'uploading', 0.8)
|
||||||
|
|||||||
@@ -1,4 +1,8 @@
|
|||||||
export type VideoProgressPhase = 'compressing' | 'uploading' | 'processing'
|
export type VideoProgressPhase =
|
||||||
|
| 'compressing'
|
||||||
|
| 'uploading'
|
||||||
|
| 'uploadingWithoutCompression'
|
||||||
|
| 'processing'
|
||||||
|
|
||||||
// Keep progress monotonic across the full client pipeline instead of showing
|
// Keep progress monotonic across the full client pipeline instead of showing
|
||||||
// three separate 0 -> 100 cycles. Backend processing progress covers the
|
// three separate 0 -> 100 cycles. Backend processing progress covers the
|
||||||
@@ -6,6 +10,7 @@ export type VideoProgressPhase = 'compressing' | 'uploading' | 'processing'
|
|||||||
const PHASE_RANGES: Record<VideoProgressPhase, [number, number]> = {
|
const PHASE_RANGES: Record<VideoProgressPhase, [number, number]> = {
|
||||||
compressing: [0, 0.3],
|
compressing: [0, 0.3],
|
||||||
uploading: [0.3, 0.5],
|
uploading: [0.3, 0.5],
|
||||||
|
uploadingWithoutCompression: [0, 0.5],
|
||||||
processing: [0.5, 1],
|
processing: [0.5, 1],
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -18,6 +23,14 @@ export function videoProgressForPhase(
|
|||||||
return start + (end - start) * clamped
|
return start + (end - start) * clamped
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function videoProgressWithinPhase(
|
||||||
|
phase: VideoProgressPhase,
|
||||||
|
progress: number,
|
||||||
|
): number {
|
||||||
|
const [start, end] = PHASE_RANGES[phase]
|
||||||
|
return Math.min(1, Math.max(0, (progress - start) / (end - start)))
|
||||||
|
}
|
||||||
|
|
||||||
export function advanceVideoProgress(
|
export function advanceVideoProgress(
|
||||||
currentProgress: number,
|
currentProgress: number,
|
||||||
phase: VideoProgressPhase,
|
phase: VideoProgressPhase,
|
||||||
|
|||||||
Reference in New Issue
Block a user