[KPhotoAlbum] new version of the videopatch

[Johannes Zarl-Zierl]

Hi Andreas,

On Donnerstag, 14. September 2017 22:38:48 CEST Andreas Schleth wrote:
> I did some more testing: Mplayer1 was somewhere on my system, not
> mplayer2 though.

Thanks for testing!

mplayer2 is (somewhat counter-intuitively) not maintained anymore. It's normal 
that you only have mplayer 1. The whole mplayer version mess was a major 
incentive for me to add ffmped support in the first place.


Commenting on the patch itself:

1. "Create 20 thumbs and take one"
// Thus it might be unnecessary to produce 20 pngs and then sift through them.

That's very much possible (at least when not using mplayer). I just copied the 
approach from the existing mplayer thumbnailing code when adding ffmpeg 
support. At this point in time, we might think loudly about dropping mplayer 
support alltogether.


2. Disabled code
Before merging, I'd like to reduce the amount of code that is commented out.
Comments in the manner "we did it this way, now we changed to this and that 
for this reason" are good, though.


3. qDebug()
In newer KPA code, we usually define a file-local debug macro (see e.g. 
BackgroundTaskManager/JobInterface.cpp).
If the qDebug() is actually meant for production code, it should probably be a 
qWarning().


Cheers,
  Johannes