Differential D22183 Diff 61780 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​bool PhononWidget::eventFilter(QObject *object, QEvent *event) {
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​ Q_UNUSED(object);
100​ if (event->type() == QEvent::MouseButtonPress){
elvisangelaccioUnsubmitted
Done

Missing space before brace

elvisangelaccio: Missing space before brace
101​ QMouseEvent *mouseEvent = static_cast<QMouseEvent*>(event);
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​ if (mouseEvent->button() == Qt::LeftButton) {
103​ // toggle playback
104​ if (m_media && m_media->state() == Phonon::State::PlayingState) {
105​ m_media->pause();
elvisangelaccioUnsubmitted
Done

Unnecessary semicolon

elvisangelaccio: Unnecessary semicolon
elvisangelaccioUnsubmitted
Done

Still not fixed ;)

elvisangelaccio: Still not fixed ;)
106​ } else {
107​ play();
108​ }
109​ return true;
110​ }
111​ }
112​ return false;
113​}
114​
98void PhononWidget::setVideoSize(const QSize& size) 115​void PhononWidget::setVideoSize(const QSize& size)
99{ 116​{
100 if (m_videoSize != size) { 117​ if (m_videoSize != size) {
101 m_videoSize = size; 118​ m_videoSize = size;
102 applyVideoSize(); 119​ applyVideoSize();
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…
103 } 120​ }
104} 121​}
105 122​
106QSize PhononWidget::videoSize() const 123​QSize PhononWidget::videoSize() const
107{ 124​{
108 return m_videoSize; 125​ return m_videoSize;
109} 126​}
110 127​
▲ Show 20 Lines • Show All 56 Lines • ▼ Show 20 Line(s)
167} 184​}
168 185​
169void PhononWidget::stateChanged(Phonon::State newstate) 186​void PhononWidget::stateChanged(Phonon::State newstate)
170{ 187​{
171 setUpdatesEnabled(false); 188​ setUpdatesEnabled(false);
172 switch (newstate) { 189​ switch (newstate) {
173 case Phonon::PlayingState: 190​ case Phonon::PlayingState:
174 case Phonon::BufferingState: 191​ case Phonon::BufferingState:
175 m_stopButton->show(); ​
176 m_playButton->hide(); 192​ m_playButton->hide();
193​ m_stopButton->show();
177 break; 194​ break;
178 default: 195​ default:
179 m_stopButton->hide(); 196​ m_stopButton->hide();
180 m_playButton->show(); 197​ m_playButton->show();
181 break; 198​ break;
182 } 199​ }
183 setUpdatesEnabled(true); 200​ setUpdatesEnabled(true);
184} 201​}
185 202​
186void PhononWidget::play() 203​void PhononWidget::play()
187{ 204​{
188 if (!m_media) { 205​ if (!m_media) {
189 m_media = new Phonon::MediaObject(this); 206​ m_media = new Phonon::MediaObject(this);
190 connect(m_media, &Phonon::MediaObject::stateChanged, 207​ connect(m_media, &Phonon::MediaObject::stateChanged,
191 this, &PhononWidget::stateChanged); 208​ this, &PhononWidget::stateChanged);
192 connect(m_media, &Phonon::MediaObject::finished, 209​ connect(m_media, &Phonon::MediaObject::finished,
193 this, &PhononWidget::finished); 210​ this, &PhononWidget::finished);
194 m_seekSlider->setMediaObject(m_media); 211​ m_seekSlider->setMediaObject(m_media);
195 } 212​ }
196 213​
197 if (!m_videoPlayer) { 214​ if (!m_videoPlayer) {
198 m_videoPlayer = new EmbeddedVideoPlayer(this); 215​ m_videoPlayer = new EmbeddedVideoPlayer(this);
216​ m_videoPlayer->setCursor(Qt::PointingHandCursor);
199 m_videoPlayer->installEventFilter(this); 217​ m_videoPlayer->installEventFilter(this);
200 m_topLayout->insertWidget(0, m_videoPlayer); 218​ 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); 219​ Phonon::createPath(m_media, m_videoPlayer);
202 applyVideoSize(); 220​ applyVideoSize();
203 } 221​ }
204 222​
205 if (!m_audioOutput) { 223​ if (!m_audioOutput) {
206 m_audioOutput = new Phonon::AudioOutput(Phonon::MusicCategory, this); 224​ m_audioOutput = new Phonon::AudioOutput(Phonon::MusicCategory, this);
207 Phonon::createPath(m_media, m_audioOutput); 225​ Phonon::createPath(m_media, m_audioOutput);
208 } 226​ }
Show All 38 Lines