Bug 29133 - Allow the platform media player to know the <video> poster URL.
Summary: Allow the platform media player to know the <video> poster URL.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-10 11:17 PDT by Andrei Popescu
Modified: 2009-10-06 04:35 PDT (History)
2 users (show)

See Also:


Attachments
Add setPoster() & canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (4.39 KB, patch)
2009-09-10 11:27 PDT, Andrei Popescu
eric: review-
Details | Formatted Diff | Diff
Add setPoster() & canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 2) (4.63 KB, patch)
2009-09-11 04:08 PDT, Andrei Popescu
no flags Details | Formatted Diff | Diff
Add prepareToPlay(), setPoster() and canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 3) (7.84 KB, patch)
2009-10-02 09:31 PDT, Andrei Popescu
no flags Details | Formatted Diff | Diff
Add prepareToPlay(), setPoster() and canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 3) (7.59 KB, patch)
2009-10-02 09:34 PDT, Andrei Popescu
eric.carlson: review+
benm: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Popescu 2009-09-10 11:17:51 PDT
On Android the media player cannot currently generate a thumbail/poster based on the frames in the video. However, if the user has specified a poster for the video, the player can use that instead. Right now, there is now way for the player to know the URL for the poster. This can be fixed by adding a 'setPoster(const String& url)' method on MediaPlayer. Will provide a patch shortly.
Comment 1 Andrei Popescu 2009-09-10 11:27:01 PDT
Created attachment 39360 [details]
Add setPoster() & canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface.
Comment 2 Eric Seidel (no email) 2009-09-10 13:29:04 PDT
Comment on attachment 39360 [details]
Add setPoster() & canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface.

Does this have any affect on anything besides andriod?  Do other <video> implementations use the poster image?  Why does android need it?  And why can't we test this?  the OOPS! in your ChangeLog will cause this to fail to land.  The reviewed by line OOPS is OK, because our landing scripts will take care of fixing that one.
Comment 3 Andrei Popescu 2009-09-10 14:06:01 PDT
Hi Eric,

(In reply to comment #2)
> (From update of attachment 39360 [details])
> Does this have any affect on anything besides andriod?  

Currently not, since Android is the only platform that currently returns true for canLoadPoster(). However, I expect that other mobile platforms may have the same problem and may need this.

>Do other <video>
> implementations use the poster image?  

Yes, all of them. It is part of the spec.

>Why does android need it?  

The problems is as follows: currently the HTMLVideoElement creates a RenderImage and loads the poster. This poster is shown on the page (where the video should render) until the media player has buffered enough content and is able to generate a thumbnail based on the first few frames of the video. 

However, this doesn't work for our player, which cannot currently generate such a thumbnail. On the other hand, if the user has specified the "poster" attribute, our player can just show the user-provided poster instead. In order to use it, it needs to know its URL. Hence this change.

>And why can't
> we test this?  

I wasn't entirely sure how to test this. As I said above, we're the only platform that returns 'true' in canLoadPoster() so all other platforms are currently unaffected by this change...Do you have a suggestion?


>the OOPS! in your ChangeLog will cause this to fail to land.

Oh, I did not know. I will just remove it if we agree what to do about testing.

Thanks,
Andrei
Comment 4 Andrei Popescu 2009-09-11 04:08:59 PDT
Created attachment 39421 [details]
Add setPoster() & canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 2)
Comment 5 Eric Carlson 2009-09-11 09:43:06 PDT
Comment on attachment 39421 [details]
Add setPoster() & canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 2)

Clearing the r? flag while I talk offline with Andrei about this.
Comment 6 Andrei Popescu 2009-10-02 09:31:01 PDT
Created attachment 40527 [details]
Add prepareToPlay(), setPoster() and canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 3)

Hi Eric,

Sorry for the late reply on this, it's only today that I got a chance to upload a new patch.

As we discussed, I have added a method that is called when the state of HTMLMediaElement is 'readyToPlay' but the media backend doesn't yet have enough data. This is the case with our backend that does not prebuffer any content. This allows the state to be correct from the beginning (NetworkState::Empty and ReadyState::HaveNothing) and allows the backend to change the network and ready states just before playback.

I left the setPoster method where it was in the first patch. Before playback, it's WebCore that draws the poster. After playback, it's the MediaPlayerPrivate that draws the poster in its paint method.

All the best,
Andrei
Comment 7 Andrei Popescu 2009-10-02 09:34:28 PDT
Created attachment 40529 [details]
Add prepareToPlay(), setPoster() and canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 3)

Looks like there was something wrong with the Changelog in the previous patch. Fixed.
Comment 8 Eric Carlson 2009-10-05 10:15:27 PDT
Comment on attachment 40529 [details]
Add prepareToPlay(), setPoster() and canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 3)

r=me

Thanks Andrei!
Comment 9 Eric Seidel (no email) 2009-10-05 11:05:50 PDT
Comment on attachment 40529 [details]
Add prepareToPlay(), setPoster() and canLoadPoster() methods to the MediaPlayer class and MediaPlayerPrivate iface. (take 3)

Andrei is not a committer yet, adding cq+.
Comment 10 Ben Murdoch 2009-10-05 11:25:43 PDT
(In reply to comment #9)
> (From update of attachment 40529 [details])
> Andrei is not a committer yet, adding cq+.

Did the CQ grab this already? Andrei asked me to remove a tab in the ChangeLog and I fixed some extra braces -- was just about to manually commit....
Comment 11 Ben Murdoch 2009-10-05 11:29:11 PDT
Looks like the CQ didn't get it yet. I'll land manually.
Comment 12 Ben Murdoch 2009-10-06 04:35:24 PDT
landed as r49164.