Summary: | [MediaFoundation] Implement seek | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||
Component: | Media | Assignee: | Alex Christensen <achristensen> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bfulgham, peavo | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Alex Christensen
2015-03-11 14:14:30 PDT
Created attachment 248450 [details]
Patch
Comment on attachment 248450 [details] 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. Comment on attachment 248450 [details] 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”. (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. (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 ... :) |