[Webkit-unassigned] [Bug 31586] [GTK] set playbin mute property depending on volume value
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Dec 29 11:29:11 PST 2009
https://bugs.webkit.org/show_bug.cgi?id=31586
Gustavo Noronha (kov) <gns at gnome.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #45274|review? |review-, commit-queue-
Flag| |
--- Comment #21 from Gustavo Noronha (kov) <gns at gnome.org> 2009-12-29 11:29:10 PST ---
(From update of attachment 45274)
WebCore/ChangeLog
1 2009-12-20 Philippe Normand <pnormand at igalia.com>
2
3 Reviewed by NOBODY (OOPS!).
4
5 [GTK] set playbin mute property depending on volume value
6 https://bugs.webkit.org/show_bug.cgi?id=31586
7
8 Adapted the other MediaPlayer implementations.,
You should write only 1 ChangeLog entry for the full change.
@@ void HTMLMediaElement::mediaPlayerTimeChanged(MediaPlayer*)
14101415 void HTMLMediaElement::mediaPlayerVolumeChanged(MediaPlayer*)
14111416 {
14121417 beginProcessingMediaPlayerCallback();
1418 if (m_player)
1419 m_volume = m_player->volume();
14131420 updateVolume();
14141421 endProcessingMediaPlayerCallback();
This looks wrong. Why are you changing this behavior?
1424 void HTMLMediaElement::mediaPlayerMuteChanged(MediaPlayer*)
1425 {
1426 beginProcessingMediaPlayerCallback();
1427 if (m_player)
1428 m_muted = m_player->muted();
1429 if (renderer())
1430 renderer()->updateFromElement();
1431 endProcessingMediaPlayerCallback();
1432 }
I don't think you should duplicate this code here. Make a setMuted call, and
make it behave like updateVolume does (checking if it is handling a player
notification, and treating it specially).
79 virtual bool supportsMuting() { return false; }
80 virtual void setMuted(bool) = 0;
supportsMuting should be have the const qualifier, like hasClosedCaptions.
457 void MediaPlayerPrivate::volumeChangedCallback()
458 {
459 double volume;
460 g_object_get(m_playBin, "volume", &volume, NULL);
461 m_player->volumeChanged(static_cast<float>(volume));
I think you are mixing unrelated changes in this patch. And this looks a bit
weird. Shouldn't you just set the volume?
I believe it would be a good idea to grab Eric Carlson and ask him about the
layering, and where you want to insert this.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list