Summary: | MediaPlayer doesn't set properties on new privates | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Otte <otte> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eric.carlson, eric | ||||||
Priority: | P2 | Keywords: | Gtk | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Benjamin Otte
2009-10-16 14:40:13 PDT
I'm just seeing there is getters already, so it looks like this is only an issue with the GStreamer elements. > I'm just seeing there is getters already, so it looks like this is only an
> issue with the GStreamer elements.
Agreed, it is the responsibility of the media engine to query the element's state when it is instantiated.
Created attachment 41825 [details]
patch
Comment on attachment 41825 [details] patch > LOG_VERBOSE(Media, "Seek: %" GST_TIME_FORMAT, GST_TIME_ARGS(sec)); > - if (!gst_element_seek( m_playBin, m_rate, > + if (!gst_element_seek( m_playBin, m_player->rate(), ^ You could take the oportunity to fix this weird space here =). > - g_object_set(G_OBJECT(m_playBin), "mute", mute, NULL); > + g_object_set(G_OBJECT(m_playBin), "volume", (double) volume, NULL); You should use static_cast<double>() here, mind the space. > @@ -673,12 +657,11 @@ void MediaPlayerPrivate::paint(GraphicsContext* context, const IntRect& rect) > > // paint the rectangle on the context and draw the surface inside. > cairo_translate(cr, rect.x() + gapWidth, rect.y() + gapHeight); > - cairo_rectangle(cr, 0, 0, rect.width(), rect.height()); > cairo_scale(cr, doublePixelAspectRatioNumerator / doublePixelAspectRatioDenominator, > doublePixelAspectRatioDenominator / doublePixelAspectRatioNumerator); > cairo_scale(cr, scale, scale); > cairo_set_source_surface(cr, src, 0, 0); > - cairo_fill(cr); > + cairo_paint(cr); > cairo_restore(cr); These changes look unrelated, can they go in a separate patch? > - void setMuted(bool); Just so it's crystal clear: this is being removed because the mute logic is already handled by HTMLMediaElement? I'll say r- mainly because of the unrelated changes above, but otherwise looks fine to me, so should be good to land after fixing the nits. Created attachment 41861 [details]
patch v2
Here's an updated patch. Sorry about the cairo_paint() - I'll file a separate bug for that.
I've had a look and setMuted() is gone from the other MediaPlayerPrivate implementations. The HTMLMediaElement takes care of it these days.
Comment on attachment 41861 [details]
patch v2
Thanks!
Comment on attachment 41861 [details]
patch v2
Rejecting patch 41861 from commit-queue.
Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11520 test cases.
svg/css/getComputedStyle-basic.xhtml -> failed
Exiting early after 1 failures. 9845 tests run.
317.97s total testing time
9844 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
6 test cases (<1%) had stderr output
Comment on attachment 41861 [details]
patch v2
This failure looks unrelated. Not sure what's going on yet.
It's a real failure, but not your fault. The bots are just way behind. bug 30098. Comment on attachment 41861 [details] patch v2 Clearing flags on attachment: 41861 Committed r50122: <http://trac.webkit.org/changeset/50122> All reviewed patches have been landed. Closing bug. |