Bug 90733

Summary: [GStreamer] cache video dimensions
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, gustavo, menard, mrobinson, pnormand, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mrobinson: review+

Philippe Normand
Reported 2012-07-07 18:03:57 PDT
There's no need to query the video-sink sink-pad caps every time in ::naturalSize(). We can simply cache this size every time the pad caps change and reuse the value by returning early from ::naturalSize().
Attachments
Patch (2.95 KB, patch)
2012-07-07 18:11 PDT, Philippe Normand
no flags
Patch (3.34 KB, patch)
2012-07-07 20:34 PDT, Philippe Normand
mrobinson: review+
Philippe Normand
Comment 1 2012-07-07 18:11:39 PDT
Martin Robinson
Comment 2 2012-07-07 18:50:16 PDT
Comment on attachment 151151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151151&action=review The logic is a bit hard to follow here. Perhaps you could do this: 1. naturalSize() would be responsible for caching. m_videoSize. 2. In notifyPlayerOfVideo when you want to invalidate the cached video size simply do: m_videoSize = IntSize(); and then let naturalSize() initialize m_videoSize lazily.
Philippe Normand
Comment 3 2012-07-07 19:54:52 PDT
(In reply to comment #2) > (From update of attachment 151151 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=151151&action=review > > The logic is a bit hard to follow here. Perhaps you could do this: > > 1. naturalSize() would be responsible for caching. m_videoSize. That's not possible unfortunately because naturalSize() is const :/ > 2. In notifyPlayerOfVideo when you want to invalidate the cached video size simply do: > > m_videoSize = IntSize(); > That I can do yes! :)
Martin Robinson
Comment 4 2012-07-07 20:10:34 PDT
(In reply to comment #3) > That's not possible unfortunately because naturalSize() is const :/ In these situations, I typically just make the cache member mutable.
Philippe Normand
Comment 5 2012-07-07 20:34:32 PDT
Philippe Normand
Comment 6 2012-07-09 19:40:55 PDT
Note You need to log in before you can comment on or make changes to this bug.