Allow dolphin to auto-play previewed media file
ClosedPublic

Authored by meven on Mar 15 2019, 12:40 PM.

Details

Summary

It is based on D19844.

I did my best to avoid glitches hence the amount of code touched.

Retry after @pekkah D7539
Moved the setting to the information panel context menu, no more timer

Settings screenshot :

This would mach the same feature in the open/save dialog (although not equivalent)

FEATURE: 378613
FIXED-IN: 19.08.0
GUI: New information panel context menu option

Test Plan

Without auto play

  • in dolphin with the information panel opened, and the auto media play feature is disabled (right on the information panel)
  • hover over media files
  • the behavior is the same as before the patch

With auto play

  • in dolphin with the information panel opened, and the auto media play feature is enabled
  • hover over media files
  • media is played automatically
  • hover over another media file, the new media is previewed

Use audio or video file as media.

Diff Detail

Repository
R318 Dolphin
Branch
arcpatch-D19782
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13215
Build 13233: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
meven marked an inline comment as done.Mar 18 2019, 1:27 PM
meven added a comment.Mar 18 2019, 1:34 PM

This is what it looks like :

Thanks, this is better now. It works and the code looks sane to me. I'd recommend changing the menu item to say "Auto-Play media files", and I'll now hand over this show to @elvisangelaccio for a more thorough code review.

Thanks again!

meven updated this revision to Diff 54225.Mar 18 2019, 2:25 PM
  • rewording
ngraham added inline comments.Mar 18 2019, 2:29 PM
src/panels/information/dolphin_informationpanelsettings.kcfg
14

Gotta change this one too

meven updated this revision to Diff 54229.Mar 18 2019, 2:46 PM
  • Fix missing rewording
meven marked an inline comment as done.Mar 18 2019, 2:47 PM
meven edited the summary of this revision. (Show Details)Mar 18 2019, 2:50 PM
ngraham accepted this revision as: ngraham.Mar 18 2019, 3:08 PM
meven added a comment.Mar 18 2019, 3:35 PM

The patch currently causes some flickering that I will address once D19844 is merged and this branch is rebased.

meven planned changes to this revision.Mar 19 2019, 12:15 PM

The patch currently causes some flickering that I will address once D19844 is merged and this branch is rebased.

meven updated this revision to Diff 54448.Mar 20 2019, 7:07 PM
  • Improve autoplay feature, add click to play/pause playback
meven retitled this revision from Allow dolphin to auto play previewed media file to Allow dolphin to auto play previewed media file, click on preview to play/pause videos or audio.Mar 20 2019, 7:13 PM
meven edited the summary of this revision. (Show Details)
meven edited the test plan for this revision. (Show Details)
meven edited the test plan for this revision. (Show Details)
meven edited the test plan for this revision. (Show Details)
meven edited the summary of this revision. (Show Details)Mar 21 2019, 10:26 AM
meven retitled this revision from Allow dolphin to auto play previewed media file, click on preview to play/pause videos or audio to BUG 378613: Allow dolphin to auto play previewed media file, click on preview to play/pause videos or audio.Mar 23 2019, 9:36 AM
meven edited the summary of this revision. (Show Details)Mar 24 2019, 5:39 PM
meven updated this revision to Diff 54701.Mar 24 2019, 6:05 PM
meven edited the test plan for this revision. (Show Details)

Rebase on master

meven updated this revision to Diff 54703.Mar 24 2019, 6:11 PM

Avoid unneeded changes

meven updated this revision to Diff 54706.Mar 24 2019, 6:22 PM

Fix building

ngraham retitled this revision from BUG 378613: Allow dolphin to auto play previewed media file, click on preview to play/pause videos or audio to Allow dolphin to auto-play previewed media file, click on preview to play/pause videos or audio.Mar 24 2019, 6:24 PM
elvisangelaccio requested changes to this revision.Mar 24 2019, 6:58 PM
elvisangelaccio added inline comments.
src/panels/information/informationpanel.cpp
216–225

Please move the elses at the end of the previous lines.

src/panels/information/phononwidget.cpp
164–177

Maybe an if-else would be nicer here.

src/panels/information/phononwidget.h
54

bool arguments in API should be avoided in favor of enums.

src/panels/information/pixmapviewer.h
76–77 ↗(On Diff #54706)

This signal is a bit redundant. The same goal (tracking left-click events) can be achieved by using an event filter in the phonon widget.

This revision now requires changes to proceed.Mar 24 2019, 6:58 PM
meven updated this revision to Diff 54718.Mar 24 2019, 7:39 PM

Use event filter to handle clicks, use enum instead of booleans, better ifs

meven updated this revision to Diff 54720.Mar 24 2019, 7:39 PM

Clean up

meven updated this revision to Diff 54721.Mar 24 2019, 7:43 PM
meven marked 3 inline comments as done.

Add comments

meven updated this revision to Diff 54723.Mar 24 2019, 7:44 PM

Clean up

meven updated this revision to Diff 54724.Mar 24 2019, 7:46 PM

Clean up

meven added a comment.Mar 24 2019, 7:48 PM

Thanks @elvisangelaccio about EventFilter I had never used it before, still getting used to to Qt Api.

meven edited the summary of this revision. (Show Details)Mar 27 2019, 8:21 AM
meven edited the summary of this revision. (Show Details)
meven added a comment.Mar 27 2019, 8:29 AM

The code in this state has one issue : when pausing the video, the video frame becomes black but the right frame can appear when moving the mouse around or resizing the window.
I don't know if this is a phonon/gstreamer kind of issue or whether or not I can fix this.
Help would be very much welcomed here.

And it has a small glitch when stopping a video preview.

meven edited the summary of this revision. (Show Details)Mar 28 2019, 7:53 AM

I didn't notice that this patch is also adding click-on-preview to toggle play/pause. To me that is an unrelated feature that should go in another commit (and will probably help you to debug it).

meven added a comment.Apr 7 2019, 8:06 PM

I didn't notice that this patch is also adding click-on-preview to toggle play/pause. To me that is an unrelated feature that should go in another commit (and will probably help you to debug it).

I planned this feature for another commit, but I got carried away since it would depend on it.
It is a quite small change, it won't take long to remove it here.

meven edited the test plan for this revision. (Show Details)Apr 7 2019, 8:23 PM
meven updated this revision to Diff 55710.Apr 7 2019, 8:30 PM

Remove click to play/pause feature

meven updated this revision to Diff 60129.Jun 20 2019, 2:07 PM

Rebasing on master

ngraham accepted this revision.Jun 20 2019, 3:38 PM

This looks fantastic to me. It works great, and the code change seems sane. Let's wait for @elvisangelaccio's review now.

src/panels/information/informationpanelcontent.h
86

typo: "eventually"

meven updated this revision to Diff 60158.Jun 20 2019, 4:57 PM

Fix typo

meven added a comment.Jun 21 2019, 3:00 PM

I have tested this under wayland with success.
In wayland the possible glitch you can encounter when stopping a video playback does not happen.

meven marked an inline comment as done.Jun 22 2019, 4:43 PM
ngraham accepted this revision.Jun 22 2019, 6:18 PM

We're in the middle of a sprint right now and I'd really like to get this committed to keep up the momentum. I say go for it, and I will accept @elvisangelaccio's wrath if any changes are later needed (though you'll be signing up to make them lol). :)

meven updated this revision to Diff 60471.Jun 23 2019, 12:18 PM

Rebasing on master

elvisangelaccio accepted this revision.Jun 23 2019, 2:36 PM

Go for it :)

This revision is now accepted and ready to land.Jun 23 2019, 2:36 PM
This revision was automatically updated to reflect the committed changes.
meven retitled this revision from Allow dolphin to auto-play previewed media file, click on preview to play/pause videos or audio to Allow dolphin to auto-play previewed media file.Jun 23 2019, 2:38 PM