RESOLVED FIXED 209341
Ensure media cache directory is created before passing to AVURLAsset.
https://bugs.webkit.org/show_bug.cgi?id=209341
Summary Ensure media cache directory is created before passing to AVURLAsset.
Jer Noble
Reported 2020-03-20 09:08:12 PDT
Ensure media cache directory is created before passing to AVURLAsset.
Attachments
Patch (2.66 KB, patch)
2020-03-20 09:13 PDT, Jer Noble
no flags
Patch (7.72 KB, patch)
2020-03-20 10:07 PDT, Jer Noble
eric.carlson: review+
Patch for landing (7.75 KB, patch)
2020-03-20 11:14 PDT, Jer Noble
no flags
Jer Noble
Comment 1 2020-03-20 09:10:11 PDT
Jer Noble
Comment 2 2020-03-20 09:13:12 PDT
Eric Carlson
Comment 3 2020-03-20 09:28:33 PDT
Comment on attachment 394089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394089&action=review > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:850 > bool usePersistentCache = player()->shouldUsePersistentCache(); > [options setObject:@(!usePersistentCache) forKey:AVURLAssetUsesNoPersistentCacheKey]; > > - if (usePersistentCache) > - [options setObject:assetCacheForPath(player()->mediaCacheDirectory()) forKey:AVURLAssetCacheKey]; > + if (usePersistentCache) { > + auto mediaCacheDirectory = player()->mediaCacheDirectory(); > + if (FileSystem::fileIsDirectory(mediaCacheDirectory, ShouldFollowSymbolicLinks::Yes) || FileSystem::makeAllDirectories(mediaCacheDirectory)) > + [options setObject:assetCacheForPath(mediaCacheDirectory) forKey:AVURLAssetCacheKey]; > + else > + ASSERT_NOT_REACHED(); > + } As we discussed, 'usePersistentCache' should be false if 'player()->mediaCacheDirectory()' is empty
Jer Noble
Comment 4 2020-03-20 10:07:32 PDT
Eric Carlson
Comment 5 2020-03-20 10:21:08 PDT
Comment on attachment 394095 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394095&action=review > Source/WebKitLegacy/mac/ChangeLog:9 > + directory by default; ensure that it's media cache directory is set during initialization. "it's" :-O > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:874 > [options setObject:@(!usePersistentCache) forKey:AVURLAssetUsesNoPersistentCacheKey]; > > - if (usePersistentCache) > - [options setObject:assetCacheForPath(player()->mediaCacheDirectory()) forKey:AVURLAssetCacheKey]; > + if (usePersistentCache) { > + if (auto* assetCache = ensureAssetCacheExistsForPath(player()->mediaCacheDirectory())) > + [options setObject:assetCache forKey:AVURLAssetCacheKey]; > + else Shouldn't we set AVURLAssetUsesNoPersistentCacheKey to NO if ensureAssetCacheExistsForPath() return nil?
Jer Noble
Comment 6 2020-03-20 11:14:13 PDT
Created attachment 394103 [details] Patch for landing
EWS
Comment 7 2020-03-20 15:36:51 PDT
Committed r258795: <https://trac.webkit.org/changeset/258795> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394103 [details].
Note You need to log in before you can comment on or make changes to this bug.