Bug 90733 - [GStreamer] cache video dimensions
Summary: [GStreamer] cache video dimensions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-07 18:03 PDT by Philippe Normand
Modified: 2012-07-09 19:40 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.95 KB, patch)
2012-07-07 18:11 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (3.34 KB, patch)
2012-07-07 20:34 PDT, Philippe Normand
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>