Dolphin - Add autoplay feature for media files
AbandonedPublic
Actions

Authored by elvisangelaccio on Aug 25 2017, 1:12 PM.

Details

Reviewers

Group Reviewers

Dolphin
KDE Applications

Summary

SUMMARY
This code patch attempts to implement a missing autoplay feature to recent versions of Dolphin file browser, for media files (videos and audio files).

TODO

As commented in the code, there are several issues which should be targeted. They are:

1) Autoplay starts (related to qtimer) even if the target file has changed before the timer has run out
2) Stop/destroy the media file preview once user does not highlight the specific item/Dolphin main window or use the window

And alternatively, these two, too (the 4th part is not described in the code) as toggleable features?

3) On/Off feature: Limit autoplay depending on file type. Autoplay should only apply to audio files, exclude video files. Handy for large movie files which do not probably need a preview feature.
4) On/Off feature: Limit autoplay by file size. Idea here is to limit the autoplay to apply only for small media files (such as desktop effect sounds) and exclude big media files such as music tracks.

Test Plan

The patch has been tested on multiple Dolphin versions, including 16.12.3, 17.04 and the latest one: 17.08. Compiles fine, works as expected.

Linux distro used: Arch Linux (64-bit).

The patch file has been applied into the program source code by slightly modifiying an existing PKGBUILD file for dolphin (added patch file as a recognizable source file). The original PKGBUILD file is available here: https://git.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/dolphin

The patch should be considered as a partial implementation. To make the full or better implementation, see TODO part in Summary field above.

User can disable or enable this feature by following these steps:

compile Dolphin from source, apply the patch file
install compiled Dolphin package
Open Dolphin, and go to Settings -> Configure Dolphin...
Open Navigation tab, and enable/disable "Play media files automatically" feature
Save your changes

Diff Detail

Repository

R318 Dolphin

Lint

Lint Skipped

Unit

Unit Tests Skipped

pekkah created this revision.Aug 25 2017, 1:12 PM

Restricted Application added subscribers: Konqueror, plasma-devel. · View Herald TranscriptAug 25 2017, 1:12 PM

pekkah edited the summary of this revision. (Show Details)Aug 25 2017, 1:12 PM

pekkah edited the summary of this revision. (Show Details)

pekkah edited the test plan for this revision. (Show Details)

pekkah edited reviewers, added: KDE Applications; removed: Plasma.Aug 25 2017, 1:15 PM

pekkah edited projects, added KDE Applications; removed Plasma.

pekkah edited subscribers, added: KDE Applications; removed: Plasma, plasma-devel.

pekkah removed a subscriber: Konqueror.

ltoscano removed a subscriber: KDE Applications.Aug 25 2017, 1:20 PM

Thanks for the contribution. Some comments after a quick test:

If I select two ore more audio files, the autoplay starts for the first selected file. This is probably unexpected.
Not sure if the new option really belongs to the Navigation section of the settings dialog.

See also the inline comments.

src/panels/information/phononwidget.cpp
23	Not needed.
86	When does this happen? Btw it could be solved by storing the file size when creating the timer, and checking the new file size in the `play()` slot.
88	Why? I would expect audio and video files to behave the same way.
95	Use the new syntax: connect(m_autoplaytimer, &QTimer::timeout, this, &PhononWidget::play) Also, the whole if block has a wrong indentation (8 spaces instead of 4).
src/settings/navigation/navigationsettingspage.cpp
47	It should probably tell how the autoplay is started (selection/hovering).

Remove all TODO's and just restart timer when selection is changed.

src/panels/information/phononwidget.cpp
92	This is local variable not class member one.

anthonyfieroni added inline comments.Aug 25 2017, 5:42 PM

src/panels/information/phononwidget.cpp
97	This should reuse some configuration not hardcoded.

I need to point out that from a security perspective this is a very dangerous change. With autoplay enabled a drive-by-download with a corrupted media file will be autoplayed when the user enters the Download order in dolphin. Chromium downloads files automatically without asking the user and stores them in ~/Downloads. This was an issue which recently hit tracker (baloo is affected in similar ways, just inside KDE nobody cared so far).

Given that I would suggest to move the media playback into a sandbox (e.g. a seccomp enabled helper process which is not allowed to write to files and fork, etc.) and only enable autoplay if there is no risk that this can be used to take over the system by just navigating to a malicious website.

In D7539#140094, @graesslin wrote:

I need to point out that from a security perspective this is a very dangerous change. With autoplay enabled a drive-by-download with a corrupted media file will be autoplayed when the user enters the Download order in dolphin. Chromium downloads files automatically without asking the user and stores them in ~/Downloads. This was an issue which recently hit tracker (baloo is affected in similar ways, just inside KDE nobody cared so far).

Given that I would suggest to move the media playback into a sandbox (e.g. a seccomp enabled helper process which is not allowed to write to files and fork, etc.) and only enable autoplay if there is no risk that this can be used to take over the system by just navigating to a malicious website.

Good point, but a sandbox is probably out of scope for Dolphin. It could be implemented in Phonon (which is what Dolphin uses to play media files), so all Phonon users would benefit from it.

cfeck added a subscriber: cfeck.Sep 1 2017, 11:57 PM

cfeck added inline comments.

src/panels/information/phononwidget.cpp
94	if(cond) -> if (cond) There is no need to add comments for obvious code. In fact, those comments lower readability instead of increasing it.
src/panels/information/phononwidget.h
82	"Type* var" -> "Type *var" lowercase -> camelCase

What's the status of this patch?

@ngraham needs some more work, marking it as such.

This revision now requires changes to proceed.Sep 30 2017, 4:47 PM

meven mentioned this in D19782: Allow dolphin to auto-play previewed media file.Mar 15 2019, 12:40 PM

meven mentioned this in R318:e6c1b97d67f6: Allow dolphin to auto-play previewed media file, click on preview to play/pause….Jun 23 2019, 2:38 PM

elvisangelaccio commandeered this revision.Jun 23 2019, 2:52 PM

elvisangelaccio edited reviewers, added: pekkah; removed: elvisangelaccio.

Restricted Application added a subscriber: kfm-devel. · View Herald TranscriptJun 23 2019, 2:52 PM

Superseded by D19782.

Revision Contents
Changeset List

			Path	Packages
M			src/panels/information/phononwidget.h (2 lines)
M			src/panels/information/phononwidget.cpp (17 lines)
M			src/settings/dolphin_generalsettings.kcfg (4 lines)
M			src/settings/navigation/navigationsettingspage.h (1 line)
M			src/settings/navigation/navigationsettingspage.cpp (9 lines)