From 0c71b098965ce4779a7da785249497c15b9d1d14 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 8 Jun 2010 11:30:05 +0100 Subject: Prevent crash when video is played without a VideoWidget The Phonon API allows video to be played via a Phonon::MediaObject, even if no Phonon::VideoWidget has been connected to it. This patch prevents the Phonon MMF backend crashing in this scenario due to dereferencing a null pointer. Reviewed-by: Thierry Bastian --- src/3rdparty/phonon/mmf/abstractvideoplayer.cpp | 10 ++++++++-- src/3rdparty/phonon/mmf/videoplayer_dsa.cpp | 7 ++++++- src/3rdparty/phonon/mmf/videoplayer_surface.cpp | 5 ++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp index 2e0ab1c..ecfce9e 100644 --- a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp @@ -211,7 +211,8 @@ void MMF::AbstractVideoPlayer::aspectRatioChanged() TRACE_CONTEXT(AbstractVideoPlayer::aspectRatioChanged, EVideoInternal); TRACE_ENTRY("state %d aspectRatio %d", state()); - updateScaleFactors(m_videoOutput->videoWindowSize()); + if (m_videoOutput) + updateScaleFactors(m_videoOutput->videoWindowSize()); TRACE_EXIT_0(); } @@ -221,7 +222,8 @@ void MMF::AbstractVideoPlayer::scaleModeChanged() TRACE_CONTEXT(AbstractVideoPlayer::scaleModeChanged, EVideoInternal); TRACE_ENTRY("state %d", state()); - updateScaleFactors(m_videoOutput->videoWindowSize()); + if (m_videoOutput) + updateScaleFactors(m_videoOutput->videoWindowSize()); TRACE_EXIT_0(); } @@ -357,6 +359,8 @@ void MMF::AbstractVideoPlayer::videoOutputChanged() void MMF::AbstractVideoPlayer::initVideoOutput() { + Q_ASSERT(m_videoOutput); + bool connected = connect( m_videoOutput, SIGNAL(videoWindowChanged()), this, SLOT(videoWindowChanged()) @@ -400,6 +404,8 @@ QSize scaleToAspect(const QSize &srcRect, int aspectWidth, int aspectHeight) void MMF::AbstractVideoPlayer::updateScaleFactors(const QSize &windowSize, bool apply) { + Q_ASSERT(m_videoOutput); + if (m_videoFrameSize.isValid()) { QRect videoRect; diff --git a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp index 1925471..f0255b1 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp @@ -61,6 +61,8 @@ DsaVideoPlayer::~DsaVideoPlayer() void MMF::DsaVideoPlayer::videoWindowScreenRectChanged() { + Q_ASSERT(m_videoOutput); + QRect windowRect = static_cast(m_videoOutput)->videoWindowScreenRect(); // Clip to physical window size @@ -130,6 +132,8 @@ void MMF::DsaVideoPlayer::createPlayer() void MMF::DsaVideoPlayer::initVideoOutput() { + Q_ASSERT(m_videoOutput); + bool connected = connect( m_videoOutput, SIGNAL(videoWindowScreenRectChanged()), this, SLOT(videoWindowScreenRectChanged()) @@ -156,7 +160,8 @@ void MMF::DsaVideoPlayer::initVideoOutput() void MMF::DsaVideoPlayer::prepareCompleted() { - videoWindowScreenRectChanged(); + if (m_videoOutput) + videoWindowScreenRectChanged(); } void MMF::DsaVideoPlayer::handleVideoWindowChanged() diff --git a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp index fda7342..5d8db26 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp @@ -59,7 +59,8 @@ SurfaceVideoPlayer::~SurfaceVideoPlayer() void MMF::SurfaceVideoPlayer::videoWindowSizeChanged() { - updateScaleFactors(m_videoOutput->videoWindowSize()); + if (m_videoOutput) + updateScaleFactors(m_videoOutput->videoWindowSize()); } @@ -80,6 +81,8 @@ void MMF::SurfaceVideoPlayer::createPlayer() void MMF::SurfaceVideoPlayer::initVideoOutput() { + Q_ASSERT(m_videoOutput); + bool connected = connect( m_videoOutput, SIGNAL(videoWindowSizeChanged()), this, SLOT(videoWindowSizeChanged()) -- cgit v0.12 From 80af9a7b09b531a849b863695b244dd1df7dfb1d Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 8 Jun 2010 14:56:31 +0100 Subject: Enable bufferStatus signal during video clip loading on NGA platforms CVideoPlayerUtility::RegisterForVideoLoadingNotification() was only called in the DSA, not the NGA, variant of the Phonon MMF backend. Task-number: QTBUG-11378 Reviewed-by: Thierry Bastian --- src/3rdparty/phonon/mmf/abstractvideoplayer.cpp | 2 ++ src/3rdparty/phonon/mmf/videoplayer_dsa.cpp | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp index ecfce9e..c45ed98 100644 --- a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp @@ -66,6 +66,8 @@ void MMF::AbstractVideoPlayer::construct() createPlayer(); + m_player->RegisterForVideoLoadingNotification(*this); + TRACE_EXIT_0(); } diff --git a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp index f0255b1..deb9774 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_dsa.cpp @@ -126,8 +126,6 @@ void MMF::DsaVideoPlayer::createPlayer() // CVideoPlayerUtility::NewL starts DSA m_dsaActive = true; - - m_player->RegisterForVideoLoadingNotification(*this); } void MMF::DsaVideoPlayer::initVideoOutput() -- cgit v0.12 From 50fbb9d19b3f524361c146de8ff0fb2fe6abacc9 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 8 Jun 2010 14:37:21 +0100 Subject: Close media clip before creating new player object Failure to Close() an existing MMF player utility object before creating a new one - which happens in the MMF backend's implementation of Phonon::MediaObject::setCurrentSource() - causes intialization of the newly-created utility to fail later on. Task-number: QTBUG-11377 Reviewed-by: Thierry Bastian --- src/3rdparty/phonon/mmf/abstractmediaplayer.h | 1 - src/3rdparty/phonon/mmf/abstractplayer.h | 1 + src/3rdparty/phonon/mmf/dummyplayer.cpp | 4 ++++ src/3rdparty/phonon/mmf/dummyplayer.h | 1 + src/3rdparty/phonon/mmf/mediaobject.cpp | 3 +++ 5 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/3rdparty/phonon/mmf/abstractmediaplayer.h b/src/3rdparty/phonon/mmf/abstractmediaplayer.h index 23a8233..7d28caf 100644 --- a/src/3rdparty/phonon/mmf/abstractmediaplayer.h +++ b/src/3rdparty/phonon/mmf/abstractmediaplayer.h @@ -71,7 +71,6 @@ protected: virtual int openFile(RFile& file) = 0; virtual int openUrl(const QString& url) = 0; virtual int bufferStatus() const = 0; - virtual void close() = 0; void updateMetaData(); virtual int numberOfMetaDataEntries() const = 0; diff --git a/src/3rdparty/phonon/mmf/abstractplayer.h b/src/3rdparty/phonon/mmf/abstractplayer.h index ab892f5..30d5243 100644 --- a/src/3rdparty/phonon/mmf/abstractplayer.h +++ b/src/3rdparty/phonon/mmf/abstractplayer.h @@ -55,6 +55,7 @@ public: AbstractPlayer(const AbstractPlayer *player); virtual void open(const Phonon::MediaSource&, RFile&) = 0; + virtual void close() = 0; // MediaObjectInterface (implemented) qint32 tickInterval() const; diff --git a/src/3rdparty/phonon/mmf/dummyplayer.cpp b/src/3rdparty/phonon/mmf/dummyplayer.cpp index 6970088..d39ef76 100644 --- a/src/3rdparty/phonon/mmf/dummyplayer.cpp +++ b/src/3rdparty/phonon/mmf/dummyplayer.cpp @@ -97,6 +97,10 @@ void MMF::DummyPlayer::open(const Phonon::MediaSource &, RFile &) } +void MMF::DummyPlayer::close() +{ + +} //----------------------------------------------------------------------------- // AbstractPlayer diff --git a/src/3rdparty/phonon/mmf/dummyplayer.h b/src/3rdparty/phonon/mmf/dummyplayer.h index 6841b5d..9d45696 100644 --- a/src/3rdparty/phonon/mmf/dummyplayer.h +++ b/src/3rdparty/phonon/mmf/dummyplayer.h @@ -58,6 +58,7 @@ public: // AbstractPlayer virtual void open(const Phonon::MediaSource&, RFile&); + virtual void close(); virtual void doSetTickInterval(qint32 interval); }; } diff --git a/src/3rdparty/phonon/mmf/mediaobject.cpp b/src/3rdparty/phonon/mmf/mediaobject.cpp index e1b921b..d264377 100644 --- a/src/3rdparty/phonon/mmf/mediaobject.cpp +++ b/src/3rdparty/phonon/mmf/mediaobject.cpp @@ -281,6 +281,9 @@ void MMF::MediaObject::createPlayer(const MediaSource &source) break; } + if (oldPlayer) + oldPlayer->close(); + AbstractPlayer* newPlayer = 0; // Construct newPlayer using oldPlayer (if not 0) in order to copy -- cgit v0.12 From 31d4037ad8e07fc4302a846271b28965f3e11069 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Tue, 8 Jun 2010 16:06:37 +0100 Subject: Fixed crash which occurs when switching between video clips When Phonon::MediaObject::setCurrentSource() is called when the MediaObject is connected to a Phonon::VideoWidget, the MMF::AbstractVideoOutput pointer is propagated inside the backend from the first MMF::AbstractVideoPlayer to the second. If the VideoWidget is subsquently re-sized, the code path enters the ScaleFactors branch of the MMF::SurfaceVideoPlayer::handleParametersChanged function. At this point, m_displayWindow is still set to the inital null value, and the assertion therefore fails. This change ensures that m_displayWindow is updated before attempting to apply the scale factor change. Task-number: QTBUG-11377 Reviewed-by: Thierry Bastian --- src/3rdparty/phonon/mmf/videoplayer_surface.cpp | 42 +++++++++++++++++-------- src/3rdparty/phonon/mmf/videoplayer_surface.h | 3 ++ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp index 5d8db26..343370c 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp @@ -118,23 +118,14 @@ void MMF::SurfaceVideoPlayer::handleParametersChanged(VideoParameters parameters if (player) { int err = KErrNone; if (parameters & WindowHandle) { - if (m_displayWindow) - player->RemoveDisplayWindow(*m_displayWindow); - - RWindow *window = static_cast(m_window); - if (window) { - window->SetBackgroundColor(TRgb(0, 0, 0, 255)); - TRAP(err, player->AddDisplayWindowL(m_wsSession, m_screenDevice, *window, rect, rect)); - if (KErrNone != err) { - setError(tr("Video display error"), err); - window = 0; - } - } - m_displayWindow = window; + removeDisplayWindow(); + addDisplayWindow(rect); } if (KErrNone == err) { if (parameters & ScaleFactors) { + if (!m_displayWindow) + addDisplayWindow(rect); Q_ASSERT(m_displayWindow); TRAP(err, player->SetVideoExtentL(*m_displayWindow, rect)); if (KErrNone == err) @@ -148,5 +139,30 @@ void MMF::SurfaceVideoPlayer::handleParametersChanged(VideoParameters parameters } } +void MMF::SurfaceVideoPlayer::addDisplayWindow(const TRect &rect) +{ + Q_ASSERT(!m_displayWindow); + RWindow *window = static_cast(m_window); + if (window) { + window->SetBackgroundColor(TRgb(0, 0, 0, 255)); + CVideoPlayerUtility2 *player = static_cast(m_player.data()); + Q_ASSERT(player); + TRAPD(err, player->AddDisplayWindowL(m_wsSession, m_screenDevice, *window, rect, rect)); + if (KErrNone == err) + m_displayWindow = window; + else + setError(tr("Video display error"), err); + } +} + +void MMF::SurfaceVideoPlayer::removeDisplayWindow() +{ + CVideoPlayerUtility2 *player = static_cast(m_player.data()); + if (player && m_displayWindow) { + player->RemoveDisplayWindow(*m_displayWindow); + m_displayWindow = 0; + } +} + QT_END_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/videoplayer_surface.h b/src/3rdparty/phonon/mmf/videoplayer_surface.h index c05da9c..8572fdc 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_surface.h +++ b/src/3rdparty/phonon/mmf/videoplayer_surface.h @@ -62,6 +62,9 @@ private: void handleVideoWindowChanged(); void handleParametersChanged(VideoParameters parameters); + void addDisplayWindow(const TRect &rect); + void removeDisplayWindow(); + private: // Window handle which has been passed to the MMF via // CVideoPlayerUtility2::SetDisplayWindowL -- cgit v0.12 From 64f711e91a30c7689314f4bceca78745236f8bee Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Thu, 17 Jun 2010 09:42:52 +0100 Subject: MMF Phonon backend: call winId() from VideoWidget constructor This is to be consistent with the backends for other platforms, which also call QWidget::winId() on the VideoWidget (or one of its children) during construction. Reviewed-by: Thierry Bastian --- src/3rdparty/phonon/mmf/abstractvideooutput.cpp | 3 ++- src/3rdparty/phonon/mmf/abstractvideoplayer.cpp | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractvideooutput.cpp b/src/3rdparty/phonon/mmf/abstractvideooutput.cpp index a8aabfd..2d221ed 100644 --- a/src/3rdparty/phonon/mmf/abstractvideooutput.cpp +++ b/src/3rdparty/phonon/mmf/abstractvideooutput.cpp @@ -60,7 +60,8 @@ MMF::AbstractVideoOutput::AbstractVideoOutput(QWidget *parent) , m_aspectRatio(DefaultAspectRatio) , m_scaleMode(DefaultScaleMode) { - + // Ensure that this widget has a native window handle + winId(); } MMF::AbstractVideoOutput::~AbstractVideoOutput() diff --git a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp index c45ed98..9ea4d18 100644 --- a/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp +++ b/src/3rdparty/phonon/mmf/abstractvideoplayer.cpp @@ -384,9 +384,6 @@ void MMF::AbstractVideoPlayer::initVideoOutput() // Suppress warnings in release builds Q_UNUSED(connected); - // Do these after all connections are complete, to ensure - // that any signals generated get to their destinations. - m_videoOutput->winId(); m_videoOutput->setVideoSize(m_videoFrameSize); } -- cgit v0.12 From 03da6bf799823d8b2f7aa8ebe337076dd5d170e4 Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 7 Jul 2010 18:12:46 +0100 Subject: Added trace statements to Phonon MMF backend Reviewed-by: trustme --- src/3rdparty/phonon/mmf/videoplayer_surface.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp index 343370c..f380e69 100644 --- a/src/3rdparty/phonon/mmf/videoplayer_surface.cpp +++ b/src/3rdparty/phonon/mmf/videoplayer_surface.cpp @@ -107,6 +107,9 @@ void MMF::SurfaceVideoPlayer::handleVideoWindowChanged() void MMF::SurfaceVideoPlayer::handleParametersChanged(VideoParameters parameters) { + TRACE_CONTEXT(SurfaceVideoPlayer::handleParametersChanged, EVideoApi); + TRACE_ENTRY("parameters 0x%x", parameters.operator int()); + TRect rect; if (m_videoOutput) { m_videoOutput->dump(); @@ -137,12 +140,20 @@ void MMF::SurfaceVideoPlayer::handleParametersChanged(VideoParameters parameters } } } + + TRACE_EXIT_0(); } void MMF::SurfaceVideoPlayer::addDisplayWindow(const TRect &rect) { + TRACE_CONTEXT(SurfaceVideoPlayer::addDisplayWindow, EVideoApi); + TRACE_ENTRY("rect %d %d - %d %d", rect.iTl.iX, rect.iTl.iY, rect.iBr.iX, rect.iBr.iY); + Q_ASSERT(!m_displayWindow); RWindow *window = static_cast(m_window); + + TRACE("window 0x%08x", window); + if (window) { window->SetBackgroundColor(TRgb(0, 0, 0, 255)); CVideoPlayerUtility2 *player = static_cast(m_player.data()); @@ -152,11 +163,17 @@ void MMF::SurfaceVideoPlayer::addDisplayWindow(const TRect &rect) m_displayWindow = window; else setError(tr("Video display error"), err); + TRACE("err %d", err); } + + TRACE_EXIT_0(); } void MMF::SurfaceVideoPlayer::removeDisplayWindow() { + TRACE_CONTEXT(SurfaceVideoPlayer::removeDisplayWindow, EVideoApi); + TRACE("player 0x%08x window 0x%08x", m_player.data(), m_displayWindow); + CVideoPlayerUtility2 *player = static_cast(m_player.data()); if (player && m_displayWindow) { player->RemoveDisplayWindow(*m_displayWindow); -- cgit v0.12 From 656fd8d3f308205c811a8b1b1c9230ce5f8fc49e Mon Sep 17 00:00:00 2001 From: Gareth Stockwell Date: Wed, 7 Jul 2010 18:43:04 +0100 Subject: Use lower case for including system header files All platform includes are now lower case, i.e. '#include ' rather than '#include '. Note that Qt includes are still camel case, e.g. '#include ' Task-number: QTBUG-6528 Reviewed-by: trustme --- src/3rdparty/phonon/mmf/abstractaudioeffect.h | 2 +- src/3rdparty/phonon/mmf/audioequalizer.cpp | 2 +- src/3rdparty/phonon/mmf/bassboost.cpp | 2 +- src/3rdparty/phonon/mmf/environmentalreverb.cpp | 2 +- src/3rdparty/phonon/mmf/loudness.cpp | 2 +- src/3rdparty/phonon/mmf/stereowidening.cpp | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/3rdparty/phonon/mmf/abstractaudioeffect.h b/src/3rdparty/phonon/mmf/abstractaudioeffect.h index 70adcf6..8879636 100644 --- a/src/3rdparty/phonon/mmf/abstractaudioeffect.h +++ b/src/3rdparty/phonon/mmf/abstractaudioeffect.h @@ -21,7 +21,7 @@ along with this library. If not, see . #include -#include +#include #include diff --git a/src/3rdparty/phonon/mmf/audioequalizer.cpp b/src/3rdparty/phonon/mmf/audioequalizer.cpp index 1d2bbd4..28433f6 100644 --- a/src/3rdparty/phonon/mmf/audioequalizer.cpp +++ b/src/3rdparty/phonon/mmf/audioequalizer.cpp @@ -16,7 +16,7 @@ along with this library. If not, see . */ -#include +#include #include "audioequalizer.h" QT_BEGIN_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/bassboost.cpp b/src/3rdparty/phonon/mmf/bassboost.cpp index 67076f6..81d9208 100644 --- a/src/3rdparty/phonon/mmf/bassboost.cpp +++ b/src/3rdparty/phonon/mmf/bassboost.cpp @@ -16,7 +16,7 @@ along with this library. If not, see . */ -#include +#include #include "bassboost.h" QT_BEGIN_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/environmentalreverb.cpp b/src/3rdparty/phonon/mmf/environmentalreverb.cpp index d4f5223..c500385 100644 --- a/src/3rdparty/phonon/mmf/environmentalreverb.cpp +++ b/src/3rdparty/phonon/mmf/environmentalreverb.cpp @@ -16,7 +16,7 @@ along with this library. If not, see . */ -#include +#include #include "environmentalreverb.h" QT_BEGIN_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/loudness.cpp b/src/3rdparty/phonon/mmf/loudness.cpp index ca05ab0..22d7518 100644 --- a/src/3rdparty/phonon/mmf/loudness.cpp +++ b/src/3rdparty/phonon/mmf/loudness.cpp @@ -16,7 +16,7 @@ along with this library. If not, see . */ -#include +#include #include "loudness.h" QT_BEGIN_NAMESPACE diff --git a/src/3rdparty/phonon/mmf/stereowidening.cpp b/src/3rdparty/phonon/mmf/stereowidening.cpp index f90651b..e452160 100644 --- a/src/3rdparty/phonon/mmf/stereowidening.cpp +++ b/src/3rdparty/phonon/mmf/stereowidening.cpp @@ -16,7 +16,7 @@ along with this library. If not, see . */ -#include +#include #include "stereowidening.h" QT_BEGIN_NAMESPACE -- cgit v0.12