Differential D22183 Diff 64533 src/panels/information/phononwidget.cpp

Changeset View

Standalone View

View Options

src/panels/information/phononwidget.cpp

Show First 20 Lines • Show All 89 Lines • ▼ Show 20 Line(s)86{
90 } 90​ }
91} 91​}
92 92​
93QUrl PhononWidget::url() const 93​QUrl PhononWidget::url() const
94{ 94​{
95 return m_url; 95​ return m_url;
96} 96​}
97 97​
98​void PhononWidget::clearUrl()
elvisangelaccioUnsubmitted
Done

Coding style: opening brace should go the next line for functions.

elvisangelaccio: Coding style: opening brace should go the next line for functions.
99​{
100​ m_url.clear();
elvisangelaccioUnsubmitted
Done

Missing space before brace

elvisangelaccio: Missing space before brace
101​}
shubhamUnsubmitted
Done

use qobjectcast

shubham: use qobjectcast
mevenAuthorUnsubmitted
Done

QEvent does not inherit QObject : qobject_cast can't be used to cast its instances.

meven: QEvent does not inherit QObject : qobject_cast can't be used to cast its instances.
102​
103​bool PhononWidget::eventFilter(QObject *object, QEvent *event)
104​{
105​ Q_UNUSED(object);
elvisangelaccioUnsubmitted
Done

Unnecessary semicolon

elvisangelaccio: Unnecessary semicolon
elvisangelaccioUnsubmitted
Done

Still not fixed ;)

elvisangelaccio: Still not fixed ;)
106​ if (event->type() == QEvent::MouseButtonPress) {
107​ const QMouseEvent *mouseEvent = static_cast<QMouseEvent*>(event);
108​ if (mouseEvent->button() == Qt::LeftButton) {
109​ // toggle playback
110​ if (m_media && m_media->state() == Phonon::State::PlayingState) {
111​ m_media->pause();
112​ } else {
113​ play();
114​ }
115​ return true;
116​ }
117​ }
118​ return false;
119​}
elvisangelaccioUnsubmitted
Done

Have you tried to override mousePressEvent() instead of using an event filter?

elvisangelaccio: Have you tried to override `mousePressEvent()` instead of using an event filter?
mevenAuthorUnsubmitted
Done

I did.
I needed to have an eventFilter so that I can reuse this code directly, informationpanelcontent.cpp line 244 :

m_preview->installEventFilter(m_phononWidget);
meven: I did. I needed to have an eventFilter so that I can reuse this code directly…
elvisangelaccioUnsubmitted
Done

Why would we need that? I just tried to move this code from eventFilter() to mousePressEvent() and it seems to work fine.

elvisangelaccio: Why would we need that? I just tried to move this code from `eventFilter()` to `mousePressEvent…
mevenAuthorUnsubmitted
Done

It is so that the preview widget can be clicked to start video playback, and thanks to this eventFilter I can reuse directly the logic for both m_phononWidget and m_preview, just use installEventFilter / removeEventFilter.
Using mousePressEvent would force me to add two mousePressEvent in PhononWidget and PixmapViewer and add a code path to call PhononWidget::play() from PixmapViewer::mousePressEvent().

Btw, my latest changes completely hide the video controls as long as the video is not playing or paused.

meven: It is so that the preview widget can be clicked to start video playback, and thanks to this…
120​
98void PhononWidget::setVideoSize(const QSize& size) 121​void PhononWidget::setVideoSize(const QSize& size)
99{ 122​{
100 if (m_videoSize != size) { 123​ if (m_videoSize != size) {
101 m_videoSize = size; 124​ m_videoSize = size;
102 applyVideoSize(); 125​ applyVideoSize();
103 } 126​ }
104} 127​}
105 128​
▲ Show 20 Lines • Show All 61 Lines • ▼ Show 20 Line(s)
167} 190​}
168 191​
169void PhononWidget::stateChanged(Phonon::State newstate) 192​void PhononWidget::stateChanged(Phonon::State newstate)
170{ 193​{
171 setUpdatesEnabled(false); 194​ setUpdatesEnabled(false);
172 switch (newstate) { 195​ switch (newstate) {
173 case Phonon::PlayingState: 196​ case Phonon::PlayingState:
174 case Phonon::BufferingState: 197​ case Phonon::BufferingState:
175 m_stopButton->show(); ​
176 m_playButton->hide(); 198​ m_playButton->hide();
199​ m_stopButton->show();
177 break; 200​ break;
178 default: 201​ default:
179 m_stopButton->hide(); 202​ m_stopButton->hide();
180 m_playButton->show(); 203​ m_playButton->show();
181 break; 204​ break;
182 } 205​ }
183 setUpdatesEnabled(true); 206​ setUpdatesEnabled(true);
184} 207​}
185 208​
186void PhononWidget::play() 209​void PhononWidget::play()
187{ 210​{
188 if (!m_media) { 211​ if (!m_media) {
189 m_media = new Phonon::MediaObject(this); 212​ m_media = new Phonon::MediaObject(this);
190 connect(m_media, &Phonon::MediaObject::stateChanged, 213​ connect(m_media, &Phonon::MediaObject::stateChanged,
191 this, &PhononWidget::stateChanged); 214​ this, &PhononWidget::stateChanged);
192 connect(m_media, &Phonon::MediaObject::finished, 215​ connect(m_media, &Phonon::MediaObject::finished,
193 this, &PhononWidget::finished); 216​ this, &PhononWidget::finished);
194 m_seekSlider->setMediaObject(m_media); 217​ m_seekSlider->setMediaObject(m_media);
195 } 218​ }
196 219​
197 if (!m_videoPlayer) { 220​ if (!m_videoPlayer) {
198 m_videoPlayer = new EmbeddedVideoPlayer(this); 221​ m_videoPlayer = new EmbeddedVideoPlayer(this);
222​ m_videoPlayer->setCursor(Qt::PointingHandCursor);
199 m_videoPlayer->installEventFilter(this); 223​ m_videoPlayer->installEventFilter(this);
200 m_topLayout->insertWidget(0, m_videoPlayer); 224​ m_topLayout->insertWidget(0, m_videoPlayer);
anthonyfieroniUnsubmitted
Done

Why 2nd time ?

anthonyfieroni: Why 2nd time ?
mevenAuthorUnsubmitted
Done

Good catch, thanks
The first line was added by mistake in a previous diff, making the one added here useless.

meven: Good catch, thanks The first line was added by mistake in a previous diff, making the one added…
201 Phonon::createPath(m_media, m_videoPlayer); 225​ Phonon::createPath(m_media, m_videoPlayer);
202 applyVideoSize(); 226​ applyVideoSize();
203 } 227​ }
204 228​
205 if (!m_audioOutput) { 229​ if (!m_audioOutput) {
206 m_audioOutput = new Phonon::AudioOutput(Phonon::MusicCategory, this); 230​ m_audioOutput = new Phonon::AudioOutput(Phonon::MusicCategory, this);
207 Phonon::createPath(m_media, m_audioOutput); 231​ Phonon::createPath(m_media, m_audioOutput);
208 } 232​ }
Show All 13 Lines
222void PhononWidget::finished() 246​void PhononWidget::finished()
223{ 247​{
224 if (m_isVideo) { 248​ if (m_isVideo) {
225 m_videoPlayer->hide(); 249​ m_videoPlayer->hide();
226 emit hasVideoChanged(false); 250​ emit hasVideoChanged(false);
227 } 251​ }
228} 252​}
229 253​
254​Phonon::State PhononWidget::state() const
255​{
256​ return m_media == nullptr ? Phonon::State::StoppedState : m_media->state();
257​}
258​
230void PhononWidget::stop() 259​void PhononWidget::stop()
231{ 260​{
232 if (m_media) { 261​ if (m_media) {
233 m_media->stop(); 262​ m_media->stop();
234 m_videoPlayer->hide(); 263​ m_videoPlayer->hide();
235 emit hasVideoChanged(false); 264​ emit hasVideoChanged(false);
236 } 265​ }
237} 266​}
Show All 9 Lines