| elvisangelaccio | |
| ngraham |
| Dolphin |
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
Without auto play
With auto play
Use audio or video file as media.
| No Linters Available |
| No Unit Test Coverage |
| Buildable 10071 | |
| Build 10089: arc lint + arc unit |
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!
| src/panels/information/dolphin_informationpanelsettings.kcfg | ||
|---|---|---|
| 14 | Gotta change this one too | |
The patch currently causes some flickering that I will address once D19844 is merged and this branch is rebased.
The patch currently causes some flickering that I will address once D19844 is merged and this branch is rebased.
| src/panels/information/informationpanel.cpp | ||
|---|---|---|
| 217–226 | Please move the elses at the end of the previous lines. | |
| src/panels/information/phononwidget.cpp | ||
| 178–191 | 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 #54718) | 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. |
Thanks @elvisangelaccio about EventFilter I had never used it before, still getting used to to Qt Api.
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.
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.
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 | ||
|---|---|---|
| 84 ↗ | (On Diff #54718) | typo: "eventually" |
I have tested this under wayland with success.
In wayland the possible glitch you can encounter when stopping a video playback does not happen.
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). :)
