142594 2015-03-11 14:14:30 -0700 [MediaFoundation] Implement seek 2015-03-23 13:33:01 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 achristensen achristensen bfulgham peavo oldest_to_newest 1076431 0 achristensen 2015-03-11 14:14:30 -0700 I found some old code from my experimenting with Media Foundation. This should probably be upstreamed, but I don't have a machine to test this on right now, and the media controls are covered up by our child window. Peavo, could you look at this? 1076443 1 248450 achristensen 2015-03-11 14:33:38 -0700 Created attachment 248450 Patch 1076479 2 248450 darin 2015-03-11 15:56:55 -0700 Comment on attachment 248450 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=248450&action=review > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:173 > + const __int64 tenMegaHertz = 10000000; Not sure this type should be __int64; given how the calculation is done, seems it should just be a double. Also, megahertz is one word, not two, so there should not be an interact. > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:182 > + if (!SUCCEEDED(m_mediaSession->Start(&GUID_NULL, &propVariant))) { > + PropVariantClear(&propVariant); > + return; > + } > + PropVariantClear(&propVariant); Why the if statement? We call PropVariantClear unconditionally. > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:187 > + const __int64 tenMegaHertz = 10000000; Again, given how it’s used it looks like this constant should be a double, not an __int64. > Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:198 > + descriptor->Release(); > + return 0; Could just set duration to 0 here and fall through rather than repeating the descriptor->Release() code twice. 1076480 3 248450 darin 2015-03-11 15:57:29 -0700 Comment on attachment 248450 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=248450&action=review >> Source/WebCore/platform/graphics/win/MediaPlayerPrivateMediaFoundation.cpp:173 >> + const __int64 tenMegaHertz = 10000000; > > Not sure this type should be __int64; given how the calculation is done, seems it should just be a double. > > Also, megahertz is one word, not two, so there should not be an interact. An “intercap”, not an “interact”. 1077441 4 peavo 2015-03-16 10:24:20 -0700 (In reply to comment #0) > I found some old code from my experimenting with Media Foundation. This > should probably be upstreamed, but I don't have a machine to test this on > right now, and the media controls are covered up by our child window. > Peavo, could you look at this? This looks good to me :) +double MediaPlayerPrivateMediaFoundation::durationDouble() const +{ + const __int64 tenMegaHertz = 10000000; + if (!m_mediaSource) + return 0; + + IMFPresentationDescriptor* descriptor; Maybe use a smart pointer here. 1077445 5 peavo 2015-03-16 10:28:06 -0700 (In reply to comment #0) > I found some old code from my experimenting with Media Foundation. This > should probably be upstreamed, but I don't have a machine to test this on > right now, and the media controls are covered up by our child window. > Peavo, could you look at this? I hope to get some time to look at how to draw directly to the WebView window soon ... :) 1079368 6 achristensen 2015-03-23 13:33:01 -0700 http://trac.webkit.org/changeset/181863 248450 2015-03-11 14:33:38 -0700 2015-03-11 15:56:55 -0700 Patch bug-142594-20150311143313.patch text/plain 3071 achristensen SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4MTQwNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDE1LTAzLTExICBBbGV4IENo cmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgW01lZGlhRm91 bmRhdGlvbl0gSW1wbGVtZW50IHNlZWsuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xNDI1OTQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3dpbi9NZWRpYVBsYXllclByaXZh dGVNZWRpYUZvdW5kYXRpb24uY3BwOgorICAgICAgICAoV2ViQ29yZTo6TWVkaWFQbGF5ZXJQcml2 YXRlTWVkaWFGb3VuZGF0aW9uOjpzZWVraW5nKToKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhUGxh eWVyUHJpdmF0ZU1lZGlhRm91bmRhdGlvbjo6c2Vla0RvdWJsZSk6CisgICAgICAgIChXZWJDb3Jl OjpNZWRpYVBsYXllclByaXZhdGVNZWRpYUZvdW5kYXRpb246OmR1cmF0aW9uRG91YmxlKToKKyAg ICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy93aW4vTWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFGb3Vu ZGF0aW9uLmg6CisKIDIwMTUtMDMtMTEgIEJlbmphbWluIFBvdWxhaW4gIDxicG91bGFpbkBhcHBs ZS5jb20+CiAKICAgICAgICAgQWRkIGJhc2ljIHN1cHBvcnQgZm9yIEJPTCBhbmQgRU9MIGFzc2Vy dGlvbnMgdG8gdGhlIFVSTCBGaWx0ZXIgcGFyc2VyCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy93aW4vTWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFGb3VuZGF0aW9uLmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy93aW4vTWVkaWFQ bGF5ZXJQcml2YXRlTWVkaWFGb3VuZGF0aW9uLmNwcAkocmV2aXNpb24gMTgxNDA2KQorKysgU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL01lZGlhUGxheWVyUHJpdmF0ZU1lZGlh Rm91bmRhdGlvbi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE2NSwxMCArMTY1LDQzIEBAIHZvaWQg TWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFGb3VuZGF0aW9uOjoKIAogYm9vbCBNZWRpYVBsYXllclBy aXZhdGVNZWRpYUZvdW5kYXRpb246OnNlZWtpbmcoKSBjb25zdAogewotICAgIG5vdEltcGxlbWVu dGVkKCk7CiAgICAgcmV0dXJuIGZhbHNlOwogfQogCit2b2lkIE1lZGlhUGxheWVyUHJpdmF0ZU1l ZGlhRm91bmRhdGlvbjo6c2Vla0RvdWJsZShkb3VibGUgdGltZSkKK3sKKyAgICBjb25zdCBfX2lu dDY0IHRlbk1lZ2FIZXJ0eiA9IDEwMDAwMDAwOworICAgIFBST1BWQVJJQU5UIHByb3BWYXJpYW50 OworICAgIFByb3BWYXJpYW50SW5pdCgmcHJvcFZhcmlhbnQpOworICAgIHByb3BWYXJpYW50LnZ0 ID0gVlRfSTg7CisgICAgcHJvcFZhcmlhbnQuaFZhbC5RdWFkUGFydCA9IHN0YXRpY19jYXN0PF9f aW50NjQ+KHRpbWUgKiB0ZW5NZWdhSGVydHopOworICAgIGlmICghU1VDQ0VFREVEKG1fbWVkaWFT ZXNzaW9uLT5TdGFydCgmR1VJRF9OVUxMLCAmcHJvcFZhcmlhbnQpKSkgeworICAgICAgICBQcm9w VmFyaWFudENsZWFyKCZwcm9wVmFyaWFudCk7CisgICAgICAgIHJldHVybjsKKyAgICB9CisgICAg UHJvcFZhcmlhbnRDbGVhcigmcHJvcFZhcmlhbnQpOworfQorCitkb3VibGUgTWVkaWFQbGF5ZXJQ cml2YXRlTWVkaWFGb3VuZGF0aW9uOjpkdXJhdGlvbkRvdWJsZSgpIGNvbnN0Cit7CisgICAgY29u c3QgX19pbnQ2NCB0ZW5NZWdhSGVydHogPSAxMDAwMDAwMDsKKyAgICBpZiAoIW1fbWVkaWFTb3Vy Y2UpCisgICAgICAgIHJldHVybiAwOworCisgICAgSU1GUHJlc2VudGF0aW9uRGVzY3JpcHRvciog ZGVzY3JpcHRvcjsKKyAgICBpZiAoIVNVQ0NFRURFRChtX21lZGlhU291cmNlLT5DcmVhdGVQcmVz ZW50YXRpb25EZXNjcmlwdG9yKCZkZXNjcmlwdG9yKSkpCisgICAgICAgIHJldHVybiAwOworICAg IAorICAgIFVJTlQ2NCBkdXJhdGlvbjsKKyAgICBpZiAoIVNVQ0NFRURFRChkZXNjcmlwdG9yLT5H ZXRVSU5UNjQoTUZfUERfRFVSQVRJT04sICZkdXJhdGlvbikpKSB7CisgICAgICAgIGRlc2NyaXB0 b3ItPlJlbGVhc2UoKTsKKyAgICAgICAgcmV0dXJuIDA7CisgICAgfQorICAgIGRlc2NyaXB0b3It PlJlbGVhc2UoKTsKKyAgICAKKyAgICByZXR1cm4gc3RhdGljX2Nhc3Q8ZG91YmxlPihkdXJhdGlv bikgLyB0ZW5NZWdhSGVydHo7Cit9CisKIGJvb2wgTWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFGb3Vu ZGF0aW9uOjpwYXVzZWQoKSBjb25zdAogewogICAgIHJldHVybiBtX3BhdXNlZDsKSW5kZXg6IFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9NZWRpYVBsYXllclByaXZhdGVNZWRp YUZvdW5kYXRpb24uaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy93aW4vTWVkaWFQbGF5ZXJQcml2YXRlTWVkaWFGb3VuZGF0aW9uLmgJKHJldmlzaW9uIDE4 MTQwNikKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9NZWRpYVBsYXll clByaXZhdGVNZWRpYUZvdW5kYXRpb24uaAkod29ya2luZyBjb3B5KQpAQCAtNTcsNiArNTcsOCBA QCBwdWJsaWM6CiAgICAgdmlydHVhbCB2b2lkIHNldFZpc2libGUoYm9vbCk7CiAKICAgICB2aXJ0 dWFsIGJvb2wgc2Vla2luZygpIGNvbnN0OworICAgIHZpcnR1YWwgdm9pZCBzZWVrRG91YmxlKGRv dWJsZSkgb3ZlcnJpZGU7CisgICAgdmlydHVhbCBkb3VibGUgZHVyYXRpb25Eb3VibGUoKSBjb25z dCBvdmVycmlkZTsKIAogICAgIHZpcnR1YWwgYm9vbCBwYXVzZWQoKSBjb25zdDsKIAo=