-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fix: Handle unsupported videos in iOS #57353
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
Merged
puneetlath
merged 29 commits into
Expensify:main
from
HezekielT:fix/ios-handle-unsupported-video-formats
Mar 7, 2025
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
d34415b
added video slash svg
HezekielT cc1484d
style change in AttachmentOfflineIndicator
HezekielT bb72f96
include video slash in Expensicons
HezekielT e9dbcd9
create a new component VideoErrorIndicator
HezekielT 33b4f6a
update in PlaybackContext
HezekielT 3369bba
handle errors generated by unsupported video inside BaseVideoPlayer
HezekielT 7aab44a
created a patch for expo-av that handles unsupported video for ios
HezekielT 3fb9c57
run prettier
HezekielT 95a183c
remove incorrect asset
HezekielT a16bf25
Merge branch 'Expensify:main' into fix/ios-handle-unsupported-video-f…
HezekielT 038cde0
added video-slash to assets/images
HezekielT 494b566
Merge branch 'Expensify:main' into fix/ios-handle-unsupported-video-f…
HezekielT 7618a3b
run prettier
HezekielT 425d489
fix eslint error
HezekielT ec82aac
remove unnecessary style changes
HezekielT 5db9b82
Add an error occurred message to en.ts and es.ts
HezekielT af9ba7e
added videoErrorText style
HezekielT d596b83
show text and icon color based on the value of isPreview passed to Vi…
HezekielT d591aeb
passed isPreview to VideoErrorIndicator
HezekielT daf4a43
run prettier
HezekielT 39aadd4
Merge branch 'main' of https://github.com/HezekielT/App into fix/ios-…
HezekielT b9a56b3
added isBuffering check to VideoErrorIndicator
HezekielT cf4f417
Merge branch 'main' of https://github.com/HezekielT/App into fix/ios-…
HezekielT de2d31d
remove unnecessary view wrapper
HezekielT 5402bed
update style
HezekielT d1c8d68
remove unnecessary style
HezekielT 4f66006
added a horizontal padding of 44 to spacing.ts
HezekielT 765e9db
Merge branch 'main' of https://github.com/HezekielT/App into fix/ios-…
HezekielT 8c1cf87
fixed styling issue in attachment offline indicator
HezekielT File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| diff --git a/node_modules/expo-av/ios/EXAV/EXAVPlayerData.m b/node_modules/expo-av/ios/EXAV/EXAVPlayerData.m | ||
| index 99dc808..01e4bb9 100644 | ||
| --- a/node_modules/expo-av/ios/EXAV/EXAVPlayerData.m | ||
| +++ b/node_modules/expo-av/ios/EXAV/EXAVPlayerData.m | ||
| @@ -158,8 +158,16 @@ NSString *const EXAVPlayerDataObserverMetadataKeyPath = @"timedMetadata"; | ||
| // unless we preload, the asset will not necessarily load the duration by the time we try to play it. | ||
| // http://stackoverflow.com/questions/20581567/avplayer-and-avfoundationerrordomain-code-11819 | ||
| EX_WEAKIFY(self); | ||
| - [avAsset loadValuesAsynchronouslyForKeys:@[ @"duration" ] completionHandler:^{ | ||
| + [avAsset loadValuesAsynchronouslyForKeys:@[ @"isPlayable", @"duration" ] completionHandler:^{ | ||
| EX_ENSURE_STRONGIFY(self); | ||
| + NSError *error = nil; | ||
| + AVKeyValueStatus status = [avAsset statusOfValueForKey:@"isPlayable" error:&error]; | ||
| + | ||
| + if (status == AVKeyValueStatusLoaded && !avAsset.isPlayable) { | ||
| + NSString *errorMessage = @"Load encountered an error: [AVAsset isPlayable:] returned false. The asset does not contains a playable content or is not supported by the device."; | ||
| + [self _finishLoadWithError:errorMessage]; | ||
| + return; | ||
| + } | ||
|
|
||
| // We prepare three items for AVQueuePlayer, so when the first finishes playing, | ||
| // second can start playing and the third can start preparing to play. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import React from 'react'; | ||
| import {View} from 'react-native'; | ||
| import Icon from '@components/Icon'; | ||
| import * as Expensicons from '@components/Icon/Expensicons'; | ||
| import Text from '@components/Text'; | ||
| import useLocalize from '@hooks/useLocalize'; | ||
| import useTheme from '@hooks/useTheme'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import variables from '@styles/variables'; | ||
|
|
||
| type VideoErrorIndicatorProps = { | ||
| /** Whether it is a preview or not */ | ||
| isPreview?: boolean; | ||
| }; | ||
|
|
||
| function VideoErrorIndicator({isPreview = false}: VideoErrorIndicatorProps) { | ||
| const theme = useTheme(); | ||
| const styles = useThemeStyles(); | ||
| const {translate} = useLocalize(); | ||
|
|
||
| return ( | ||
| <View style={[styles.flexColumn, styles.alignItemsCenter, styles.justifyContentCenter, styles.pAbsolute, styles.h100, styles.w100]}> | ||
| <Icon | ||
| fill={isPreview ? theme.border : theme.icon} | ||
| src={Expensicons.VideoSlash} | ||
| width={variables.eReceiptEmptyIconWidth} | ||
| height={variables.eReceiptEmptyIconWidth} | ||
| /> | ||
| {!isPreview && ( | ||
| <View> | ||
| <Text style={[styles.notFoundTextHeader, styles.ph11]}>{translate('common.errorOccuredWhileTryingToPlayVideo')}</Text> | ||
| </View> | ||
| )} | ||
| </View> | ||
| ); | ||
| } | ||
|
|
||
| VideoErrorIndicator.displayName = 'VideoErrorIndicator'; | ||
|
|
||
| export default VideoErrorIndicator; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
shouldShowErrorTextis more generic. What do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can go with
shouldShowErrorTextbutisPreviewis also used inVideoErrorIndicatorin order to determine the color of video slash icon when it is in preview and in attachment modal so if we replace it withshouldShowErrorTextit might become confusing.Also
AttachmentOfflineIndicatorusesisPreviewas the name while showing text.Based on this, do you think we should keep using it as
isPreviewor should we still update itshouldShowErrorTextThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer
shouldShowErrorTextbut seems we need to keep isPreview to keep consistency with AttachmentOfflineIndicator