Skip to content

Commit

Permalink
🙃 Asset download improvement (Joystream#5932)
Browse files Browse the repository at this point in the history
* Remove old prefetch logic

* Improve current asset URL resolver
  • Loading branch information
WRadoslaw committed Feb 27, 2024
1 parent aaf7235 commit 61ba811
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { SvgActionLinkUrl, SvgIllustrativePlay } from '@/assets/icons'
import { Pill } from '@/components/Pill'
import { absoluteRoutes } from '@/config/routes'
import { useClipboard } from '@/hooks/useClipboard'
import { useVideoPreload } from '@/hooks/useVideoPreload'
import { useVideoTileSharedLogic } from '@/hooks/useVideoTileSharedLogic'
import { SentryLogger } from '@/utils/logs'
import { formatDurationShort } from '@/utils/time'
Expand All @@ -20,24 +19,15 @@ type VideoTileViewerProps = {
detailsVariant?: VideoDetailsVariant
direction?: 'vertical' | 'horizontal'
className?: string
prefetch?: boolean
}

export const VideoTileViewer: FC<VideoTileViewerProps> = ({
id,
onClick,
prefetch,
detailsVariant,
direction,
className,
}) => {
export const VideoTileViewer: FC<VideoTileViewerProps> = ({ id, onClick, detailsVariant, direction, className }) => {
const navigate = useNavigate()
const { video, loading } = useBasicVideo(id ?? '', {
skip: !id,
onError: (error) => SentryLogger.error('Failed to fetch video', 'VideoTile', error, { video: { id } }),
})
const { copyToClipboard } = useClipboard()
useVideoPreload(prefetch ? video?.media?.resolvedUrls : undefined)

const { avatarPhotoUrls, isLoadingAvatar, isLoadingThumbnail, thumbnailPhotoUrls, videoHref } =
useVideoTileSharedLogic(video)
Expand Down
28 changes: 20 additions & 8 deletions packages/atlas/src/hooks/useGetAssetUrl.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react'
import { useEffect, useRef, useState } from 'react'

import { atlasConfig } from '@/config'
import { AssetTestOptions, logDistributorPerformance, testAssetDownload } from '@/providers/assets/assets.helpers'
Expand Down Expand Up @@ -35,7 +35,7 @@ export const getSingleAssetUrl = async (
dataObjectType: type || undefined,
resolvedUrl: distributionAssetUrl,
}
const assetTestPromise = testAssetDownload(distributionAssetUrl, type, opts)
const [assetTestPromise, cleanup] = testAssetDownload(distributionAssetUrl, type, opts)
const assetTestPromiseWithTimeout = withTimeout(
assetTestPromise,
timeout ?? atlasConfig.storage.assetResponseTimeout
Expand All @@ -52,6 +52,7 @@ export const getSingleAssetUrl = async (

return distributionAssetUrl
} catch (err) {
cleanup?.()
if (err instanceof MediaError) {
let codec = ''
if (type === 'video' && !mobile) {
Expand All @@ -75,7 +76,7 @@ export const getSingleAssetUrl = async (
return new Promise((res) => {
const promises: Promise<string>[] = []
for (const distributionAssetUrl of urls) {
const assetTestPromise = testAssetDownload(distributionAssetUrl, type)
const [assetTestPromise] = testAssetDownload(distributionAssetUrl, type)
promises.push(assetTestPromise)
}

Expand All @@ -99,17 +100,32 @@ export const getSingleAssetUrl = async (
export const useGetAssetUrl = (urls: string[] | undefined | null, type: AssetType | null, opts?: AssetTestOptions) => {
const [url, setUrl] = useState<string | undefined>(undefined)
const [isLoading, setIsLoading] = useState(true)
const assetPromise = useRef<Promise<string | undefined> | null>(null)
const { userBenchmarkTime } = useOperatorsContext()
const id = urls?.[0]?.split('/').pop()
useEffect(() => {
// nothing should be done if old promise is still pending
if (assetPromise.current) {
return
}

if (!urls || (url && urls.includes(url)) || (!url && !urls.length)) {
setIsLoading(false)
return
}

const init = async () => {
setUrl(undefined)
setIsLoading(true)
const resolvedUrl = await getSingleAssetUrl(urls, id, type, userBenchmarkTime.current ?? undefined, opts)
assetPromise.current = getSingleAssetUrl(
urls,
id,
type,
type === 'video' ? 10_000 : userBenchmarkTime.current ?? undefined,
opts
)
const resolvedUrl = await assetPromise.current
assetPromise.current = null

setIsLoading(false)
if (resolvedUrl) {
Expand All @@ -120,10 +136,6 @@ export const useGetAssetUrl = (urls: string[] | undefined | null, type: AssetTyp
}

init()

return () => {
setIsLoading(false)
}
}, [id, opts, type, url, urls, userBenchmarkTime])

return { url, isLoading }
Expand Down
4 changes: 0 additions & 4 deletions packages/atlas/src/hooks/useVideoPreload.ts

This file was deleted.

39 changes: 25 additions & 14 deletions packages/atlas/src/providers/assets/assets.helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,24 @@ export type AssetTestOptions = {
resolveOnlyOnEvents?: (keyof HTMLVideoElementEventMap)[]
}

export const testAssetDownload = (url: string, type: AssetType | null, opts?: AssetTestOptions): Promise<string> => {
return new Promise((_resolve, _reject) => {
export const testAssetDownload = (
url: string,
type: AssetType | null,
opts?: AssetTestOptions
): [Promise<string>, (() => void) | null] => {
let img: HTMLImageElement | null = null
let video: HTMLVideoElement | null = null
let cleanup: (() => void) | null = null
const videoEvents: (keyof HTMLVideoElementEventMap)[] = opts?.resolveOnlyOnEvents ?? [
'loadedmetadata',
'loadeddata',
'canplay',
'progress',
]
const assetPromise = new Promise<string>((_resolve, _reject) => {
const isImageType = type && ['thumbnail', 'avatar', 'cover'].includes(type)
let img: HTMLImageElement | null = null
let video: HTMLVideoElement | null = null
const videoEvents: (keyof HTMLVideoElementEventMap)[] = opts?.resolveOnlyOnEvents ?? [
'loadedmetadata',
'loadeddata',
'canplay',
'progress',
]

const cleanup = () => {

cleanup = () => {
if (img) {
img.removeEventListener('error', reject)
img.removeEventListener('load', resolve)
Expand All @@ -44,19 +49,23 @@ export const testAssetDownload = (url: string, type: AssetType | null, opts?: As
videoEvents.forEach((event) => {
video?.removeEventListener(event, resolve)
})
video.pause()
video.preload = 'none'
video.setAttribute('src', '')
video.load()
video.remove()
video = null
}
}

const resolve = () => {
cleanup()
cleanup?.()

_resolve(url)
}

const reject = (err?: unknown) => {
cleanup()
cleanup?.()
_reject(err)
}

Expand Down Expand Up @@ -92,6 +101,8 @@ export const testAssetDownload = (url: string, type: AssetType | null, opts?: As
reject()
}
})

return [assetPromise, cleanup]
}

export const logDistributorPerformance = async (assetUrl: string, eventEntry: DistributorEventEntry) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const CuratorHomepage = () => {
contentProps={{
type: 'grid',
grid: DEFAULT_VIDEO_GRID,
children: tiles?.map((video, idx) => <VideoTileViewer id={video.id} key={idx} prefetch={idx < 10} />),
children: tiles?.map((video, idx) => <VideoTileViewer id={video.id} key={idx} />),
}}
footerProps={{
reachedEnd: !pageInfo?.hasNextPage,
Expand Down
2 changes: 1 addition & 1 deletion packages/atlas/src/views/viewer/HomeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const HomeView: FC = () => {
contentProps={{
type: 'grid',
grid: DEFAULT_VIDEO_GRID,
children: tiles?.map((video, idx) => <VideoTileViewer id={video.id} key={idx} prefetch={idx < 10} />),
children: tiles?.map((video, idx) => <VideoTileViewer id={video.id} key={idx} />),
}}
footerProps={{
reachedEnd: !hasMoreVideos,
Expand Down
3 changes: 0 additions & 3 deletions packages/atlas/src/views/viewer/VideoView/MoreVideos.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type MoreVideosProps = {
categoryName?: string | null
videoId?: string
type: 'channel' | 'category'
shouldPrefetch?: boolean
}

const NUMBER_OF_VIDEOS = 6
Expand All @@ -34,7 +33,6 @@ export const MoreVideos: FC<MoreVideosProps> = ({
categoryName,
videoId,
type,
shouldPrefetch,
}) => {
const videoCategories = categoryId ? displayCategoriesLookup[categoryId].videoCategories : undefined
const where =
Expand Down Expand Up @@ -66,7 +64,6 @@ export const MoreVideos: FC<MoreVideosProps> = ({
id={video.id}
detailsVariant="withChannelName"
direction={lgMatch ? 'horizontal' : 'vertical'}
prefetch={idx < 5 && shouldPrefetch}
/>
</GridItem>
))}
Expand Down
26 changes: 4 additions & 22 deletions packages/atlas/src/views/viewer/VideoView/VideoView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ export const VideoView: FC = () => {
)
const [isInView, ref] = useIntersectionObserver()
const [isCommenting, setIsCommenting] = useState<boolean>(false)
const [canPrefetchNew, setCanPrefetchNew] = useState(false)

const mdMatch = useMediaMatch('md')
const { addVideoView } = useAddVideoView()
Expand Down Expand Up @@ -170,9 +169,6 @@ export const VideoView: FC = () => {
// eslint-disable-next-line react-hooks/exhaustive-deps
const handleTimeUpdate = useCallback(
throttle((time) => {
if (!canPrefetchNew && time > 5_000) {
setCanPrefetchNew(true)
}
if (video?.id) {
updateWatchedVideos('INTERRUPTED', video.id, time)
}
Expand Down Expand Up @@ -283,7 +279,7 @@ export const VideoView: FC = () => {
</>
)}
</PlayerGridItem>
{!isCinematic && <SideItems video={video} loading={loading} canStartPrefetch={canPrefetchNew} />}
{!isCinematic && <SideItems video={video} loading={loading} />}
</PlayerWrapper>
</PlayerGridWrapper>
<LimitedWidthContainer>
Expand All @@ -304,23 +300,15 @@ export const VideoView: FC = () => {
/>
)}
</GridItem>
<SideItems video={video} loading={loading} canStartPrefetch={canPrefetchNew} />
<SideItems video={video} loading={loading} />
</LayoutGrid>
)}
</LimitedWidthContainer>
</>
)
}

const SideItems = ({
video,
loading,
canStartPrefetch,
}: {
video: ReturnType<typeof useFullVideo>['video']
loading: boolean
canStartPrefetch: boolean
}) => {
const SideItems = ({ video, loading }: { video: ReturnType<typeof useFullVideo>['video']; loading: boolean }) => {
const { id } = useParams()
const { openNftPutOnSale, openNftAcceptBid, openNftChangePrice, openNftPurchase, openNftSettlement, cancelNftSale } =
useNftActions()
Expand Down Expand Up @@ -359,13 +347,7 @@ const SideItems = ({
onWithdrawBid={(bid, createdAt) => id && createdAt && bid && withdrawBid(id, bid, createdAt)}
/>
)}
<MoreVideos
channelId={channelId}
channelName={channelName}
videoId={id}
type="channel"
shouldPrefetch={canStartPrefetch}
/>
<MoreVideos channelId={channelId} channelName={channelName} videoId={id} type="channel" />
{belongsToCategories?.map((category) => (
<MoreVideos
key={category.id}
Expand Down

0 comments on commit 61ba811

Please sign in to comment.