-
Notifications
You must be signed in to change notification settings - Fork 289
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
Video cache #2191
Draft
jb55
wants to merge
2
commits into
master
Choose a base branch
from
video-cache
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Video cache #2191
Conversation
This file contains 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 commit implements a simple but functional video cache. It works by providing a method called `maybe_cached_url`, where a video URL can be passed in, and this method will either return the URL of a cached version of this video if available, or the original URL if not. It also downloads new video URLs on the background into the cache folder for use next time. Functional testing ------------------- PASS Device: iPhone 15 simulator iOS: 17.4 Damus: Approximately this commit Setup: - Debug connection - Expiry time locally changed to 5 minutes Steps: 1. Basic functionality 1. Go to a profile with lots of videos 2. Scroll down 3. Filter logs to only logs that start with "Loading video with URL" 4. Check that most videos are being loaded from external URLs. PASS 5. Now restart the app and go to that same profile 6. Scroll down and watch logs. Videos should now be loaded with an internal file URL. PASS 2. Automatic cache refresh after expiry 1. Go to the video-heavy profile, make note of the external URL. 2. Go to a different screen and then come back to that video. Make sure the file was loaded from cache. PASS 3. Now go to a different screen and wait 5 minutes. 4. Come back to the same video. It should be loaded from the external URL. PASS 3. "Clear cache" button functionality 1. Go to the video-heavy profile, make note of the external URL. 2. Go to a different screen and then come back to that video. Make sure the file was loaded from cache. PASS 3. Now quit the app (to ensure file is not in use when trying to delete it) 4. Clear cache in settings 5. Go back to the same video. It should now be loaded from the external URL. PASS Performance testing ----------------------- Device: iPhone 13 mini iOS: 17.3.1 Damus: This commit Baseline: 87de88861adb3b41d73998452e7c876ab5ee06bf Setup: - Debug connection - Expiry time locally changed to 5 minutes - Running on Profile mode, with XCode Instruments Steps: 1. Start recording network activity with XCode Instruments 2. Go to a video-heavy profile (e.g. Julian Figueroa) 3. Scroll down to a specific video (Make sure to scroll through at least 5 videos) 4. Stop recording and measure the "Bytes In" from "Network connections" 5. Repeat this for all test configurations Results: - Baseline (No caching support): 26.74 MiB - This commit (First run, cleared cache): 40.52 MiB - This commit (Second run, cache filled with videos): 8.13 MiB Automated test coverage ------------------------ PASS Device: iPhone 15 simulator iOS: 17.4 Damus: This commit Coverage: - Ran new automated tests multiple times. PASS 3/3 times - Ran all other automated tests. PASS Signed-off-by: Daniel D’Aquino <[email protected]> Link: [email protected] Signed-off-by: William Casarin <[email protected]>
This commit brings significant improvements to the video cache feature. Previously, the cache would merely download the video when requested, in parallel with AVPlayer which also triggers a video download. The video cache has been updated to tap into the AVPlayer loading process, removing the download duplication. Here is how that works: 1. The player requests an AVAsset from the cache. 2. The cache will return a cached asset if possible, or a special AVURLAsset with a custom `AVAssetResourceLoaderDelegate`. 3. The video player will start sending loading requests to this loader delegate. 4. Upon receiving the first request, the loader delegate begins to download the video data on the background. 5. Upon receiving these requests, the loader delegate will also record the requests, so that it can serve them once possible 6. The loader delegate keeps track of all video data chunks as it receives them from the download task, through the `URLSessionDataDelegate` and `URLSessionTaskDelegate` protocols 7. As it receives data, it checks all pending loading requests from the AVPlayer, and fulfills them as soon as possible 8. If the download fails (e.g. timeout errors, loss of connection), it attempts to restart the download. 9. If the download succeeds, it saves the video to the cache on disk Closes: #1717 Changelog-Added: Add video cache to save network bandwidth Signed-off-by: Daniel D’Aquino <[email protected]> Link: [email protected] Signed-off-by: William Casarin <[email protected]>
This was referenced May 6, 2024
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This resumes the video cache work, I merged it too early since the tests were broken