138149 2014-10-28 14:22:24 -0700 [EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time 2014-12-16 13:28:26 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jer.noble jer.noble commit-queue eric.carlson glenn philipj sergio oldest_to_newest 1044611 0 jer.noble 2014-10-28 14:22:24 -0700 [EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time 1044614 1 240571 jer.noble 2014-10-28 14:33:22 -0700 Created attachment 240571 Patch 1044632 2 240571 bfulgham 2014-10-28 16:06:46 -0700 Comment on attachment 240571 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240571&action=review Looks good. I had a couple of minor questions about some failure cases. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:695 > + if (!getAVStreamSessionClass() || ![getAVStreamSessionClass() instancesRespondToSelector:@selector(initWithStorageDirectoryAtURL:)]) Did we used to handle AVF media streams that did not support initWithStorageDirectoryAtURL? It looks like the answer is no, but just to be careful I'll ask! :-) > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:699 > + m_streamSession = adoptNS([[getAVStreamSessionClass() alloc] initWithStorageDirectoryAtURL:[NSURL fileURLWithPath:sessionStorageDirectory()]]); Does anything bad happen if we call NSURL fileURLWithPath: <empty string>? 1044633 3 240571 jer.noble 2014-10-28 16:13:15 -0700 Comment on attachment 240571 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240571&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:695 >> + if (!getAVStreamSessionClass() || ![getAVStreamSessionClass() instancesRespondToSelector:@selector(initWithStorageDirectoryAtURL:)]) > > Did we used to handle AVF media streams that did not support initWithStorageDirectoryAtURL? It looks like the answer is no, but just to be careful I'll ask! :-) AVMediaStream didn't used to support initWithStorageDirectoryAtURL, instead it had a setStorageDirectoryAtURL:error: method, but we did check that they responder to that selector for before calling. >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:699 >> + m_streamSession = adoptNS([[getAVStreamSessionClass() alloc] initWithStorageDirectoryAtURL:[NSURL fileURLWithPath:sessionStorageDirectory()]]); > > Does anything bad happen if we call NSURL fileURLWithPath: <empty string>? Interesting. I don't think so, but for that to happen, confstr() would have to return NULL, which is itself exceptional. I'll address this in a follow-up patch, where we pull the storage location out of ChromeClient rather than using _CS_DARWIN_USER_CACHE_DIR. 1044644 4 jer.noble 2014-10-28 16:51:43 -0700 Committed r175284: <http://trac.webkit.org/changeset/175284> 1056106 5 jer.noble 2014-12-16 13:28:26 -0800 <rdar://problem/19269057> 240571 2014-10-28 14:33:22 -0700 2014-10-28 16:06:46 -0700 Patch bug-138149-20141028143330.patch text/plain 6052 jer.noble U3VidmVyc2lvbiBSZXZpc2lvbjogMTc1MjY3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzM3ZDBkZTM0ZjBkNDJm M2QxMmU0M2IyZmZjN2IyZDlmNjMwMDM2OC4uMTRhMTExNzdhYzExNDZkMTkyMzdmNDkzM2M2Nzhh OWZiNzkwZTQ2ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIyIEBACiAyMDE0LTEwLTI4ICBKZXIg Tm9ibGUgIDxqZXIubm9ibGVAYXBwbGUuY29tPgogCisgICAgICAgIFtFTUVdW01hY10gQWRvcHQg bmV3IEFWU3RyZWFtU2Vzc2lvbiBBUEk6IHBhc3Mgc3RvcmFnZURpcmVjdG9yeUF0VVJMIGF0IGNy ZWF0aW9uLXRpbWUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEzODE0OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIEFWU3RyZWFtU2Vzc2lvbiBub3cgcmVxdWlyZXMgaXRzIHN0b3JhZ2UgbG9jYXRpb24gYXQg Y3JlYXRpb24tdGltZSwgcmF0aGVyIHRoYW4gcG9zdC1jcmVhdGlvbi4gVGhpcyByZXF1aXJlcwor ICAgICAgICB1cyB0byBtb3ZlIHN0b3JhZ2UgbG9jYXRpb24gY29kZSBmcm9tIENETVNlc3Npb25N ZWRpYVNvdXJjZUFWRk9iakMgdG8gTWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFTb3VyY2VBVkZPYmpD LgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvYXZmb3VuZGF0aW9uL29iamMvQ0RNU2Vz c2lvbk1lZGlhU291cmNlQVZGT2JqQy5tbToKKyAgICAgICAgKFdlYkNvcmU6OkNETVNlc3Npb25N ZWRpYVNvdXJjZUFWRk9iakM6OnVwZGF0ZSk6CisgICAgICAgIChXZWJDb3JlOjpzZXNzaW9uU3Rv cmFnZURpcmVjdG9yeSk6IERlbGV0ZWQuCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvYXZm b3VuZGF0aW9uL29iamMvTWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFTb3VyY2VBVkZPYmpDLm1tOgor ICAgICAgICAoV2ViQ29yZTo6c2Vzc2lvblN0b3JhZ2VEaXJlY3RvcnkpOgorICAgICAgICAoV2Vi Q29yZTo6TWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFTb3VyY2VBVkZPYmpDOjpzdHJlYW1TZXNzaW9u KToKKworMjAxNC0xMC0yOCAgSmVyIE5vYmxlICA8amVyLm5vYmxlQGFwcGxlLmNvbT4KKwogICAg ICAgICBSRUdSRVNTSU9OKHIxNzE1OTMpIFtNYWNdIE1lZGlhIGNvbnRyb2xzIGNyZWF0ZSBhIGxh cmdlIChhbmQgdW5uZWNlc3NhcnkpIGJhY2tpbmcgc3RvcmUKICAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzNzc1NwogCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9hdmZvdW5kYXRpb24vb2JqYy9DRE1TZXNzaW9uTWVk aWFTb3VyY2VBVkZPYmpDLm1tIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvYXZm b3VuZGF0aW9uL29iamMvQ0RNU2Vzc2lvbk1lZGlhU291cmNlQVZGT2JqQy5tbQppbmRleCA0MTAw ZmQxNzg2MTU3MzZiMjRlNzg4NzgxNThjODc2OGJlZjc1OWZkLi4xN2VhYmZiNDFkODMzNmViMGNl NWM2Y2Q2MTk5NGFjZTg1OTBlNzBmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9hdmZvdW5kYXRpb24vb2JqYy9DRE1TZXNzaW9uTWVkaWFTb3VyY2VBVkZPYmpD Lm1tCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2F2Zm91bmRhdGlvbi9v YmpjL0NETVNlc3Npb25NZWRpYVNvdXJjZUFWRk9iakMubW0KQEAgLTU1LDcgKzU1LDYgQEAgU09G VF9MSU5LX0NPTlNUQU5UX01BWV9GQUlMKEFWRm91bmRhdGlvbiwgQVZTdHJlYW1TZXNzaW9uQ29u dGVudFByb3RlY3Rpb25TZXNzaW8KIEBlbmQKIAogQGludGVyZmFjZSBBVlN0cmVhbVNlc3Npb24g OiBOU09iamVjdAotLSAoQk9PTClzZXRTdG9yYWdlRGlyZWN0b3J5QXRVUkw6KE5TVVJMICopc3Rv cmFnZVVSTCBhcHBJZGVudGlmaWVyOihOU0RhdGEgKilhcHBJZGVudGlmaWVyIGVycm9yOihOU0Vy cm9yICoqKW91dEVycm9yOwogLSAodm9pZClhZGRTdHJlYW1EYXRhUGFyc2VyOihBVlN0cmVhbURh dGFQYXJzZXIgKilzdHJlYW1EYXRhUGFyc2VyOwogLSAodm9pZClyZW1vdmVTdHJlYW1EYXRhUGFy c2VyOihBVlN0cmVhbURhdGFQYXJzZXIgKilzdHJlYW1EYXRhUGFyc2VyOwogLSAodm9pZClleHBp cmU7CkBAIC0xNzUsMjIgKzE3NCw2IEBAIHN0YXRpYyBib29sIGlzRXF1YWwoVWludDhBcnJheSog ZGF0YSwgY29uc3QgY2hhciogbGl0ZXJhbCkKICAgICByZXR1cm4gIWxpdGVyYWxbbGVuZ3RoXTsK IH0KIAotc3RhdGljIGNvbnN0IFN0cmluZyYgc2Vzc2lvblN0b3JhZ2VEaXJlY3RvcnkoKQotewot ICAgIHN0YXRpYyBOZXZlckRlc3Ryb3llZDxTdHJpbmc+IHNlc3Npb25EaXJlY3RvcnlQYXRoOwot Ci0gICAgaWYgKHNlc3Npb25EaXJlY3RvcnlQYXRoLmdldCgpLmlzRW1wdHkoKSkgewotICAgICAg ICBjaGFyIGNhY2hlRGlyZWN0b3J5UGF0aFtQQVRIX01BWF07Ci0gICAgICAgIGlmICghY29uZnN0 cihfQ1NfREFSV0lOX1VTRVJfQ0FDSEVfRElSLCBjYWNoZURpcmVjdG9yeVBhdGgsIFBBVEhfTUFY KSkKLSAgICAgICAgICAgIHJldHVybiBXVEY6OmVtcHR5U3RyaW5nKCk7Ci0KLSAgICAgICAgc2Vz c2lvbkRpcmVjdG9yeVBhdGguZ2V0KCkuYXBwZW5kKFN0cmluZyhjYWNoZURpcmVjdG9yeVBhdGgs IHN0cmxlbihjYWNoZURpcmVjdG9yeVBhdGgpKSk7Ci0gICAgICAgIHNlc3Npb25EaXJlY3RvcnlQ YXRoLmdldCgpLmFwcGVuZChBU0NJSUxpdGVyYWwoIkFWU3RyZWFtU2Vzc2lvbi8iKSk7Ci0gICAg fQotCi0gICAgcmV0dXJuIHNlc3Npb25EaXJlY3RvcnlQYXRoLmdldCgpOwotfQotCiBib29sIENE TVNlc3Npb25NZWRpYVNvdXJjZUFWRk9iakM6OnVwZGF0ZShVaW50OEFycmF5KiBrZXksIFJlZlB0 cjxVaW50OEFycmF5PiYgbmV4dE1lc3NhZ2UsIHVuc2lnbmVkIHNob3J0JiBlcnJvckNvZGUsIHVu c2lnbmVkIGxvbmcmIHN5c3RlbUNvZGUpCiB7CiAgICAgaWYgKG1fbW9kZSA9PSBLZXlSZWxlYXNl KQpAQCAtMjI3LDEyICsyMTAsNiBAQCBib29sIENETVNlc3Npb25NZWRpYVNvdXJjZUFWRk9iakM6 OnVwZGF0ZShVaW50OEFycmF5KiBrZXksIFJlZlB0cjxVaW50OEFycmF5PiYgbgogCiAgICAgaWYg KHNob3VsZEdlbmVyYXRlS2V5UmVxdWVzdCkgewogICAgICAgICBSZXRhaW5QdHI8TlNEYXRhPiBj ZXJ0aWZpY2F0ZURhdGEgPSBhZG9wdE5TKFtbTlNEYXRhIGFsbG9jXSBpbml0V2l0aEJ5dGVzOm1f Y2VydGlmaWNhdGUtPmRhdGEoKSBsZW5ndGg6bV9jZXJ0aWZpY2F0ZS0+bGVuZ3RoKCldKTsKLSAg ICAgICAgaWYgKG1fc3RyZWFtU2Vzc2lvbiAmJiBbbV9zdHJlYW1TZXNzaW9uIHJlc3BvbmRzVG9T ZWxlY3RvcjpAc2VsZWN0b3Ioc2V0U3RvcmFnZURpcmVjdG9yeUF0VVJMOnN0b3JhZ2VVUkw6YXBw SWRlbnRpZmllcjplcnJvcjopXSkgewotICAgICAgICAgICAgW21fc3RyZWFtU2Vzc2lvbiBzZXRT dG9yYWdlRGlyZWN0b3J5QXRVUkw6W05TVVJMIGZpbGVVUkxXaXRoUGF0aDpzZXNzaW9uU3RvcmFn ZURpcmVjdG9yeSgpXSBhcHBJZGVudGlmaWVyOmNlcnRpZmljYXRlRGF0YS5nZXQoKSBlcnJvcjpu aWxdOwotICAgICAgICAgICAgZm9yIChhdXRvJiBzb3VyY2VCdWZmZXIgOiBtX3NvdXJjZUJ1ZmZl cnMpCi0gICAgICAgICAgICAgICAgW21fc3RyZWFtU2Vzc2lvbiBhZGRTdHJlYW1EYXRhUGFyc2Vy OnNvdXJjZUJ1ZmZlci0+cGFyc2VyKCldOwotICAgICAgICAgICAgTE9HKE1lZGlhLCAiQ0RNU2Vz c2lvbk1lZGlhU291cmNlQVZGT2JqQzo6dXBkYXRlKCVwKSAtIGNyZWF0ZWQgc3RyZWFtIHNlc3Np b24gJXAiLCB0aGlzLCBtX3N0cmVhbVNlc3Npb24uZ2V0KCkpOwotICAgICAgICB9CiAKICAgICAg ICAgaWYgKG1fc291cmNlQnVmZmVycy5pc0VtcHR5KCkpCiAgICAgICAgICAgICByZXR1cm4gdHJ1 ZTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2F2Zm91bmRh dGlvbi9vYmpjL01lZGlhUGxheWVyUHJpdmF0ZU1lZGlhU291cmNlQVZGT2JqQy5tbSBiL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2F2Zm91bmRhdGlvbi9vYmpjL01lZGlhUGxheWVy UHJpdmF0ZU1lZGlhU291cmNlQVZGT2JqQy5tbQppbmRleCBmMGViNjFjYWE4ZTA0ZmU3N2I4YTc2 ZGY1NWIwYzBhMTgwZTI2YjE4Li43YTEyZTgxNjU3ZmMyYjlhMDBiMTZjNGQ0MmZjZDRlMTNmMjY3 NDhiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9hdmZvdW5k YXRpb24vb2JqYy9NZWRpYVBsYXllclByaXZhdGVNZWRpYVNvdXJjZUFWRk9iakMubW0KKysrIGIv U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvYXZmb3VuZGF0aW9uL29iamMvTWVkaWFQ bGF5ZXJQcml2YXRlTWVkaWFTb3VyY2VBVkZPYmpDLm1tCkBAIC0xMTYsNiArMTE2LDEzIEBAIFNP RlRfTElOS19DT05TVEFOVChDb3JlTWVkaWEsIGtDTVRpbWViYXNlTm90aWZpY2F0aW9uX0VmZmVj dGl2ZVJhdGVDaGFuZ2VkLCBDRlN0CiAtICh2b2lkKXJlbW92ZVRpbWVPYnNlcnZlcjooaWQpb2Jz ZXJ2ZXI7CiBAZW5kCiAKKyNwcmFnbWEgbWFyayAtIAorI3ByYWdtYSBtYXJrIEFWU3RyZWFtU2Vz c2lvbgorCitAaW50ZXJmYWNlIEFWU3RyZWFtU2Vzc2lvbiA6IE5TT2JqZWN0CistIChpbnN0YW5j ZXR5cGUpaW5pdFdpdGhTdG9yYWdlRGlyZWN0b3J5QXRVUkw6KE5TVVJMICopc3RvcmFnZURpcmVj dG9yeTsKK0BlbmQKKwogbmFtZXNwYWNlIFdlYkNvcmUgewogCiAjcHJhZ21hIG1hcmsgLQpAQCAt NjY3LDEwICs2NzQsMjkgQEAgdm9pZCBNZWRpYVBsYXllclByaXZhdGVNZWRpYVNvdXJjZUFWRk9i akM6OnNpemVDaGFuZ2VkKCkKIH0KIAogI2lmIEVOQUJMRShFTkNSWVBURURfTUVESUFfVjIpCitz dGF0aWMgY29uc3QgU3RyaW5nJiBzZXNzaW9uU3RvcmFnZURpcmVjdG9yeSgpCit7CisgICAgc3Rh dGljIE5ldmVyRGVzdHJveWVkPFN0cmluZz4gc2Vzc2lvbkRpcmVjdG9yeVBhdGg7CisKKyAgICBp ZiAoc2Vzc2lvbkRpcmVjdG9yeVBhdGguZ2V0KCkuaXNFbXB0eSgpKSB7CisgICAgICAgIGNoYXIg Y2FjaGVEaXJlY3RvcnlQYXRoW1BBVEhfTUFYXTsKKyAgICAgICAgaWYgKCFjb25mc3RyKF9DU19E QVJXSU5fVVNFUl9DQUNIRV9ESVIsIGNhY2hlRGlyZWN0b3J5UGF0aCwgUEFUSF9NQVgpKQorICAg ICAgICAgICAgcmV0dXJuIFdURjo6ZW1wdHlTdHJpbmcoKTsKKworICAgICAgICBzZXNzaW9uRGly ZWN0b3J5UGF0aC5nZXQoKS5hcHBlbmQoU3RyaW5nKGNhY2hlRGlyZWN0b3J5UGF0aCwgc3RybGVu KGNhY2hlRGlyZWN0b3J5UGF0aCkpKTsKKyAgICAgICAgc2Vzc2lvbkRpcmVjdG9yeVBhdGguZ2V0 KCkuYXBwZW5kKEFTQ0lJTGl0ZXJhbCgiQVZTdHJlYW1TZXNzaW9uLyIpKTsKKyAgICB9CisgICAg CisgICAgcmV0dXJuIHNlc3Npb25EaXJlY3RvcnlQYXRoLmdldCgpOworfQorCiBBVlN0cmVhbVNl c3Npb24qIE1lZGlhUGxheWVyUHJpdmF0ZU1lZGlhU291cmNlQVZGT2JqQzo6c3RyZWFtU2Vzc2lv bigpCiB7CisgICAgaWYgKCFnZXRBVlN0cmVhbVNlc3Npb25DbGFzcygpIHx8ICFbZ2V0QVZTdHJl YW1TZXNzaW9uQ2xhc3MoKSBpbnN0YW5jZXNSZXNwb25kVG9TZWxlY3RvcjpAc2VsZWN0b3IoaW5p dFdpdGhTdG9yYWdlRGlyZWN0b3J5QXRVUkw6KV0pCisgICAgICAgIHJldHVybiBuaWw7CisKICAg ICBpZiAoIW1fc3RyZWFtU2Vzc2lvbikKLSAgICAgICAgbV9zdHJlYW1TZXNzaW9uID0gYWRvcHRO UyhbW2dldEFWU3RyZWFtU2Vzc2lvbkNsYXNzKCkgYWxsb2NdIGluaXRdKTsKKyAgICAgICAgbV9z dHJlYW1TZXNzaW9uID0gYWRvcHROUyhbW2dldEFWU3RyZWFtU2Vzc2lvbkNsYXNzKCkgYWxsb2Nd IGluaXRXaXRoU3RvcmFnZURpcmVjdG9yeUF0VVJMOltOU1VSTCBmaWxlVVJMV2l0aFBhdGg6c2Vz c2lvblN0b3JhZ2VEaXJlY3RvcnkoKV1dKTsKICAgICByZXR1cm4gbV9zdHJlYW1TZXNzaW9uLmdl dCgpOwogfQogCg==