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+

Description Philippe Normand 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().
Comment 1 Philippe Normand 2012-07-07 18:11:39 PDT
Created attachment 151151 [details]
Patch
Comment 2 Martin Robinson 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.
Comment 3 Philippe Normand 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! :)
Comment 4 Martin Robinson 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.
Comment 5 Philippe Normand 2012-07-07 20:34:32 PDT
Created attachment 151153 [details]
Patch
Comment 6 Philippe Normand 2012-07-09 19:40:55 PDT
Committed r122187: <http://trac.webkit.org/changeset/122187>