|Summary:||MediaPlayer doesn't set properties on new privates|
|Product:||WebKit||Reporter:||Benjamin Otte <otte>|
|Severity:||Normal||CC:||commit-queue, eric.carlson, eric|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Benjamin Otte 2009-10-16 14:40:13 PDT
Comment 1 Benjamin Otte 2009-10-16 14:42:12 PDT
I'm just seeing there is getters already, so it looks like this is only an issue with the GStreamer elements.
Comment 2 Eric Carlson 2009-10-22 06:31:24 PDT
> 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.
Comment 3 Benjamin Otte 2009-10-25 13:27:36 PDT
Created attachment 41825 [details] patch
Comment 4 Gustavo Noronha (kov) 2009-10-25 17:36:17 PDT
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.
Comment 5 Benjamin Otte 2009-10-26 04:44:08 PDT
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 6 Gustavo Noronha (kov) 2009-10-26 15:37:56 PDT
Comment on attachment 41861 [details] patch v2 Thanks!
Comment 7 WebKit Commit Bot 2009-10-26 15:58:11 PDT
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 8 Eric Seidel (no email) 2009-10-26 16:19:40 PDT
Comment on attachment 41861 [details] patch v2 This failure looks unrelated. Not sure what's going on yet.
Comment 9 Eric Seidel (no email) 2009-10-26 16:23:45 PDT
It's a real failure, but not your fault. The bots are just way behind. bug 30098.
Comment 10 WebKit Commit Bot 2009-10-26 19:42:55 PDT
Comment on attachment 41861 [details] patch v2 Clearing flags on attachment: 41861 Committed r50122: <http://trac.webkit.org/changeset/50122>
Comment 11 WebKit Commit Bot 2009-10-26 19:42:59 PDT
All reviewed patches have been landed. Closing bug.