Skip to content
Closed
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
1 change: 1 addition & 0 deletions android/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<uses-permission android:name="android.permission.ACCESS_MEDIA_LOCATION"/>
<uses-permission android:name="android.permission.CAMERA"/>
<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.MODIFY_AUDIO_SETTINGS"/>
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.RECORD_AUDIO"/>
<uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW"/>
Expand Down
9 changes: 8 additions & 1 deletion app.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@
"recordAudioAndroid": true
}
],
[
"expo-audio",
{
"microphonePermission": "$(PRODUCT_NAME) needs microphone access to record audio with your videos and manage audio sessions. This allows the app to capture your voice narration during recordings and handle interruptions from phone calls or other apps gracefully.",
"recordAudioAndroid": true
}
],
Comment on lines +65 to +71
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both expo-camera and expo-audio plugins are configured with microphone permissions. This creates duplicate permission requests and may lead to confusion. Since expo-camera already handles both camera and microphone permissions for video recording (lines 57-63), the expo-audio plugin's microphone permission configuration is redundant. The expo-audio package should only be used for audio session management (setAudioModeAsync), not for requesting permissions. Consider removing the microphonePermission and recordAudioAndroid settings from the expo-audio plugin configuration, or add a comment explaining why duplicate permission configurations are necessary.

Suggested change
[
"expo-audio",
{
"microphonePermission": "$(PRODUCT_NAME) needs microphone access to record audio with your videos and manage audio sessions. This allows the app to capture your voice narration during recordings and handle interruptions from phone calls or other apps gracefully.",
"recordAudioAndroid": true
}
],
"expo-audio",

Copilot uses AI. Check for mistakes.
"expo-video",
"expo-web-browser"
],
Expand All @@ -76,4 +83,4 @@
},
"owner": "morepriyam"
}
}
}
107 changes: 106 additions & 1 deletion app/(camera)/shorts.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import CameraControls from "@/components/CameraControls";
import CloseButton from "@/components/CloseButton";
import UploadCloseButton from "@/components/UploadCloseButton";
import RecordButton from "@/components/RecordButton";
import RecordButton, { RecordButtonRef } from "@/components/RecordButton";
import RecordingProgressBar, {
RecordingSegment,
} from "@/components/RecordingProgressBar";
Expand All @@ -26,12 +26,15 @@
import * as React from "react";
import {
Alert,
AppState,
AppStateStatus,
Platform,
StyleSheet,
TouchableOpacity,
View,
} from "react-native";
import { useFocusEffect } from "@react-navigation/native";
import { setIsAudioActiveAsync } from "expo-audio";
import { DraftStorage } from "@/utils/draftStorage";
import { fileStore } from "@/utils/fileStore";
import {
Expand Down Expand Up @@ -81,6 +84,7 @@
storeConfig();
}, [server, token]);
const cameraRef = React.useRef<CameraView>(null);
const recordButtonRef = React.useRef<RecordButtonRef>(null);

// Use a stable ref callback to avoid CameraView remounting on every render
// This prevents the camera from being recreated on each state update
Expand Down Expand Up @@ -154,12 +158,12 @@
useDerivedValue(() => {
// Read all shared values to create listeners
// This prevents warnings when values are updated from JS
isHoldRecording.value;

Check warning on line 161 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

Expected an assignment or function call and instead saw an expression
recordingModeShared.value;

Check warning on line 162 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

Expected an assignment or function call and instead saw an expression
currentZoom.value;

Check warning on line 163 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

Expected an assignment or function call and instead saw an expression
savedZoom.value;

Check warning on line 164 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

Expected an assignment or function call and instead saw an expression
currentTouchY.value;

Check warning on line 165 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

Expected an assignment or function call and instead saw an expression
initialTouchY.value;

Check warning on line 166 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

Expected an assignment or function call and instead saw an expression
return 0; // Return dummy value
});

Expand Down Expand Up @@ -264,8 +268,83 @@
isHoldRecording.value = false;
recordingModeShared.value = "";
}
}, [isRecording]);

Check warning on line 271 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

React Hook React.useEffect has missing dependencies: 'isHoldRecording' and 'recordingModeShared'. Either include them or remove the dependency array

// Track previous app state for Android (to detect genuine background transitions)
const appStateRef = React.useRef<AppStateStatus>(AppState.currentState);
const lastBackgroundTimeRef = React.useRef<number>(0);

// Handle app state changes during recording (background/foreground interruptions)
React.useEffect(() => {
if (!isRecording) return;

const handleAppStateChange = async (nextAppState: AppStateStatus) => {
const prevState = appStateRef.current;

// On Android, ignore rapid state changes (less than 500ms in background)
// This prevents false triggers from permission dialogs, file pickers, etc.
if (Platform.OS === "android") {
if (nextAppState === "background" || nextAppState === "inactive") {
lastBackgroundTimeRef.current = Date.now();
} else if (nextAppState === "active" && prevState !== "active") {
const timeInBackground = Date.now() - lastBackgroundTimeRef.current;
if (timeInBackground < 500) {
console.log(`[ShortsScreen] Ignoring rapid state change (${timeInBackground}ms in background)`);
appStateRef.current = nextAppState;
return;
}
}
}

console.log("[ShortsScreen] AppState:", prevState, "->", nextAppState);

// Only act on genuine transitions
if (prevState === nextAppState) {
return;
}

if (nextAppState === "background" || nextAppState === "inactive") {
// Stop recording immediately when app goes to background
if (cameraRef.current) {
try {
cameraRef.current.stopRecording();
// Disable audio when going to background
await setIsAudioActiveAsync(false);
} catch (error) {
console.warn("[ShortsScreen] Error stopping recording on background:", error);
}
}
Comment on lines +308 to +316
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential race condition when the app goes to background. The code calls both cameraRef.current.stopRecording() and setIsAudioActiveAsync(false) in a try-catch block, but doesn't await the stopRecording() call before disabling audio. Since stopRecording() is async (it waits for the recording to finish), the audio session might be disabled while the camera is still completing the recording. Consider awaiting stopRecording or restructuring the order of operations to ensure proper cleanup sequence.

Copilot uses AI. Check for mistakes.
} else if (nextAppState === "active" && prevState.match(/inactive|background/)) {
// Re-enable audio when app becomes active again (only from genuine background)
try {
await setIsAudioActiveAsync(true);
} catch (error) {
console.warn("[ShortsScreen] Error re-enabling audio:", error);
}
Comment on lines +311 to +323
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of setIsAudioActiveAsync(false/true) to enable/disable audio when the app goes to background/foreground is not a standard pattern for expo-audio. The expo-audio package's setIsAudioActiveAsync function is typically used to activate/deactivate the audio subsystem for playback scenarios. For recording interruption handling, the audio session configuration via setAudioModeAsync (which is already done in RecordButton.tsx) should be sufficient. Calling setIsAudioActiveAsync(false) when going to background and setIsAudioActiveAsync(true) when returning to foreground may cause unintended side effects or conflicts with the audio session already configured for recording. Consider whether these calls are actually necessary, or document the specific reasoning for this pattern.

Copilot uses AI. Check for mistakes.

// Reset all animations and button states to initial state
recordButtonRef.current?.reset();

// Reset zoom and touch states
setZoom(0);
savedZoom.value = 0;
currentZoom.value = 0;
setScreenTouchActive(false);
isHoldRecording.value = false;
recordingModeShared.value = "";

console.log("[ShortsScreen] 🔄 Reset all animations and states");
}

appStateRef.current = nextAppState;
};

// Use 'focus' event on Android to avoid false triggers, 'change' on iOS
const eventType = Platform.OS === "android" ? "focus" : "change";
const subscription = AppState.addEventListener(eventType, handleAppStateChange);
return () => subscription.remove();
}, [isRecording]);

Check warning on line 346 in app/(camera)/shorts.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

React Hook React.useEffect has missing dependencies: 'currentZoom', 'isHoldRecording', 'recordingModeShared', and 'savedZoom'. Either include them or remove the dependency array
Comment thread
adithya1012 marked this conversation as resolved.
Comment on lines +278 to +346
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AppState effect's handleAppStateChange function references several values from the component scope (savedZoom, currentZoom, setZoom, setScreenTouchActive, isHoldRecording, recordingModeShared) but these are not included in the effect's dependency array. While these are mostly refs and shared values that are stable, this could potentially cause stale closures. However, since the effect is recreated whenever isRecording changes, and these values are refs/shared values, this is likely fine. Consider adding a comment explaining why these dependencies are not included.

Copilot uses AI. Check for mistakes.
Comment on lines +278 to +346
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AppState effect only runs when isRecording is true, but the cleanup function removes the subscription whenever the effect re-runs or unmounts. If isRecording becomes false while the app is in the background (which can happen when recording is stopped), the effect will unmount and clean up the subscription. When isRecording becomes true again, a new subscription is created. However, consider that if recording stops while in background, and the app returns to active state, the reset logic in lines 317-337 won't run because the effect isn't active (isRecording is false). This means UI states might not reset properly. Consider adding a separate useEffect that always monitors app state (not just during recording) to handle the reset logic when returning from background, regardless of recording state.

Copilot uses AI. Check for mistakes.

useFocusEffect(
React.useCallback(() => {
// On Android, force camera remount ONLY when returning from a screen that uses video
Expand Down Expand Up @@ -353,6 +432,29 @@
updateVideoStabilizationMode(mode);
};

const handleCameraMountError = (error: { message: string }) => {
console.error("[ShortsScreen] ❌ Camera mount error:", error);
Alert.alert(
"Camera Error",
"Failed to start camera. Please try again.",
[
{
text: "Retry",
onPress: () => {
console.log("[ShortsScreen] 🔄 Retrying camera mount...");
// Force camera remount
setCameraKey((prev) => prev + 1);
},
},
{ text: "OK" },
]
);
};

const handleCameraReady = () => {
console.log("[ShortsScreen] ✅ Camera ready");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of logging normal state unless it's in the bug

};

const handlePreview = () => {
if (currentDraftId && recordingSegments.length > 0) {
// Mark that we need to remount camera when returning (video player will be used)
Expand Down Expand Up @@ -574,6 +676,8 @@
facing={cameraFacing}
enableTorch={torchEnabled}
zoom={zoom}
onMountError={handleCameraMountError}
onCameraReady={handleCameraReady}
{...(Platform.OS === "ios"
? {
videoStabilizationMode: mapToNativeVideoStabilization(
Expand Down Expand Up @@ -642,6 +746,7 @@
</View>

<RecordButton
ref={recordButtonRef}
cameraRef={cameraRef}
maxDuration={180}
totalDuration={maxDurationLimitSeconds}
Expand Down
81 changes: 75 additions & 6 deletions components/RecordButton.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { CameraView } from "expo-camera";
import { setAudioModeAsync } from "expo-audio";
import * as React from "react";
import { Animated, StyleSheet, TouchableOpacity, View } from "react-native";
import VideoConcatModule from "@/modules/video-concat";

export interface RecordButtonRef {
reset: () => void;
}

interface RecordButtonProps {
cameraRef: React.RefObject<CameraView | null>;
maxDuration?: number;
Expand All @@ -26,7 +31,7 @@
screenTouchActive?: boolean;
}

export default function RecordButton({
const RecordButton = React.forwardRef<RecordButtonRef, RecordButtonProps>(({
cameraRef,
maxDuration = 180,
onRecordingStart,
Expand All @@ -40,7 +45,7 @@
onButtonTouchStart,
onButtonTouchEnd,
screenTouchActive = false,
}: RecordButtonProps) {
}, ref) => {
const [isRecording, setIsRecording] = React.useState(false);
const [recordingMode, setRecordingMode] = React.useState<
"tap" | "hold" | null
Expand Down Expand Up @@ -105,9 +110,23 @@
}
}, [screenTouchActive, buttonInitiatedRecording, isRecording, recordingMode]); // eslint-disable-line react-hooks/exhaustive-deps

const startRecording = (mode: "tap" | "hold") => {
const startRecording = async (mode: "tap" | "hold") => {
if (!cameraRef.current || isRecording || remainingTime <= 0) return;

// Configure audio session for better interruption handling
try {
await setAudioModeAsync({
allowsRecording: true,
playsInSilentMode: true,
interruptionMode: "doNotMix", // Request exclusive audio focus
shouldPlayInBackground: false, // Don't play in background
});
console.log("[RecordButton] ✅ Audio session configured successfully");
} catch (error) {
console.warn("[RecordButton] ⚠️ Failed to configure audio session:", error);
// Continue with recording even if audio session config fails
}
Comment on lines +116 to +128
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When setAudioModeAsync fails, the code logs a warning but continues with recording. However, this means the recording will proceed without proper audio session configuration, which defeats the purpose of this PR (handling audio interruptions). Consider whether recording should be prevented or if the user should be alerted when audio session configuration fails, especially if this is a critical requirement for proper interruption handling. Alternatively, add a comment explaining that recording can proceed safely without audio session configuration.

Copilot uses AI. Check for mistakes.

setIsRecording(true);
setRecordingMode(mode);
setButtonInitiatedRecording(true);
Expand Down Expand Up @@ -180,8 +199,17 @@
.catch(async (error) => {
const recordingDuration = (Date.now() - recordingStartTimeRef.current) / 1000;

if (!error.message?.includes("stopped")) {
console.log("[RecordButton] Recording failed");
// Check for specific error types
if (
error.message?.includes("interrupted") ||
error.message?.includes("stopped") ||
error.message?.includes("background") ||
error.message?.includes("cancelled")
) {
console.warn("[RecordButton] ⚠️ Recording interrupted (expected):", error.message);
// Don't show error alert for expected interruptions
} else {
console.error("[RecordButton] ❌ Recording failed (unexpected):", error);
}
Comment on lines +202 to 213
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling logic checks for multiple error message patterns using string inclusion checks. This approach is fragile as it depends on specific error message text which could vary across platforms or expo-camera versions. Consider checking for error types/codes instead of message strings if expo-camera provides them, or document the expected error messages for different scenarios. Additionally, the current logic silently ignores interruption errors without providing any feedback mechanism to the parent component, which might need to know that recording was interrupted versus manually stopped.

Copilot uses AI. Check for mistakes.
onRecordingComplete?.(null, mode, recordingDuration);
return null;
Expand Down Expand Up @@ -245,6 +273,43 @@
}).start();
};

// Reset all animations and states to initial values
const reset = React.useCallback(() => {
// Stop any ongoing animations
if (pulsingRef.current) {
pulsingRef.current.stop();
}
if (progressIntervalRef.current) {
clearInterval(progressIntervalRef.current);
progressIntervalRef.current = null;
}
if (holdTimeoutRef.current) {
clearTimeout(holdTimeoutRef.current);
holdTimeoutRef.current = null;
}

// Reset animation values to initial state
scaleAnim.setValue(1);
borderRadiusAnim.setValue(30);
outerBorderScaleAnim.setValue(1);

// Reset state
setIsHoldingForRecord(false);
setIsRecording(false);
setRecordingMode(null);
setButtonInitiatedRecording(false);
isHoldingRef.current = false;
manuallyStoppedRef.current = false;
recordingPromiseRef.current = null;
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reset function doesn't reset all state that might be affected during recording. Specifically, it doesn't reset the recordingStartTimeRef which tracks when recording started. If the app is backgrounded and then foregrounded, and recording starts again without this being reset, duration calculations could be incorrect. Consider resetting recordingStartTimeRef to 0 or Date.now() to ensure clean state.

Suggested change
recordingPromiseRef.current = null;
recordingPromiseRef.current = null;
recordingStartTimeRef.current = 0;

Copilot uses AI. Check for mistakes.

console.log("[RecordButton] 🔄 Reset all animations and states");
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reset function in useCallback has an empty dependency array, but it references state setters (setIsHoldingForRecord, setIsRecording, setRecordingMode, setButtonInitiatedRecording) and animated values (scaleAnim, borderRadiusAnim, outerBorderScaleAnim). While the animated values and refs don't need to be in the dependency array (they're stable references), and state setters are also stable, the empty array is technically correct. However, for clarity and to follow React best practices, consider adding a comment explaining why the dependency array is empty, since ESLint rules often flag this pattern.

Suggested change
console.log("[RecordButton] 🔄 Reset all animations and states");
console.log("[RecordButton] 🔄 Reset all animations and states");
// Dependency array is intentionally empty: this callback only uses stable
// refs, Animated values, and React state setters, which do not change.

Copilot uses AI. Check for mistakes.
}, []);

Check warning on line 306 in components/RecordButton.tsx

View workflow job for this annotation

GitHub Actions / Lint and Test

React Hook React.useCallback has missing dependencies: 'borderRadiusAnim', 'outerBorderScaleAnim', and 'scaleAnim'. Either include them or remove the dependency array

// Expose reset method via ref
React.useImperativeHandle(ref, () => ({
reset,
}));

const stopRecording = async () => {
if (!cameraRef.current || !isRecording) return;

Expand Down Expand Up @@ -420,7 +485,11 @@
</View>
</View>
);
}
});

RecordButton.displayName = "RecordButton";

export default RecordButton;

const styles = StyleSheet.create({
buttonContainer: {
Expand Down
6 changes: 6 additions & 0 deletions ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ PODS:
- Yoga
- ExpoAsset (11.1.7):
- ExpoModulesCore
- ExpoAudio (0.4.9):
- ExpoModulesCore
- ExpoBlur (14.1.5):
- ExpoModulesCore
- ExpoCamera (16.1.11):
Expand Down Expand Up @@ -2199,6 +2201,7 @@ DEPENDENCIES:
- EXImageLoader (from `../node_modules/expo-image-loader/ios`)
- Expo (from `../node_modules/expo`)
- ExpoAsset (from `../node_modules/expo-asset/ios`)
- ExpoAudio (from `../node_modules/expo-audio/ios`)
- ExpoBlur (from `../node_modules/expo-blur/ios`)
- ExpoCamera (from `../node_modules/expo-camera/ios`)
- ExpoCrypto (from `../node_modules/expo-crypto/ios`)
Expand Down Expand Up @@ -2323,6 +2326,8 @@ EXTERNAL SOURCES:
:path: "../node_modules/expo"
ExpoAsset:
:path: "../node_modules/expo-asset/ios"
ExpoAudio:
:path: "../node_modules/expo-audio/ios"
ExpoBlur:
:path: "../node_modules/expo-blur/ios"
ExpoCamera:
Expand Down Expand Up @@ -2524,6 +2529,7 @@ SPEC CHECKSUMS:
EXImageLoader: 4d3d3284141f1a45006cc4d0844061c182daf7ee
Expo: dd7f49380001a55ea776f905f8b3c1c540160b09
ExpoAsset: ef06e880126c375f580d4923fdd1cdf4ee6ee7d6
ExpoAudio: d98f922b17a945cd33f8462d086278dbd810c537
ExpoBlur: 3c8885b9bf9eef4309041ec87adec48b5f1986a9
ExpoCamera: e1879906d41184e84b57d7643119f8509414e318
ExpoCrypto: a9f1d75baeea6ef8b03c1660621585196c382e85
Expand Down
12 changes: 12 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"@react-navigation/elements": "^2.3.8",
"@react-navigation/native": "^7.1.6",
"expo": "~53.0.23",
"expo-audio": "~0.4.9",
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package version specified (~0.4.9) differs from what's mentioned in the PR description (~1.1.1). The actual version being installed is 0.4.9 according to package-lock.json, which is correct. However, the PR description should be updated to reflect the accurate version number to avoid confusion.

Copilot uses AI. Check for mistakes.
"expo-blur": "~14.1.5",
"expo-camera": "~16.1.11",
"expo-constants": "~17.1.6",
Expand Down
Loading