Review Request: Add ffmpegthumbnailer for video previews

[Christoph Feck]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3818/#review5481
-----------------------------------------------------------


What is the status here?

I have seen no decision from kde-multimedia to add libffmpegthumbnailer as an external dependency, or you did not communicate that clearly.

>From your recent changes it looks like you opted to add the code to the KDE repository instead. This may be necessary if upstream libffmpegthumbnailer author cannot guarantee a stable binary interface yet.

In this case, you should be willing to maintain it (handle bug reports and fix them, clean up the code etc.) but the current patch still includes much unneeded and unused code (if you would keep the external library, it would be shared with other applications).

You should show your commitment to maintainership by removing at least the totally unnecessary "image data->PNG file->image data" step; this would be a good way to get to know the code so that you can do other cleanups later.

Remember that on May 11th is both dependency freeze as well as feature freeze.

PS: Don't misunderstand me, I am using kffmpegthumbnailer from the day it appeared on kde-apps, and prefer it over mplayerthumbnails, so I am all for getting it in. Just not in its current state.

- Christoph


On 2010-05-01 12:57:38, Andreas Scherf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3818/
> -----------------------------------------------------------
> 
> (Updated 2010-05-01 12:57:38)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> This change adds a ffmpegbackend for rendering video thumbnails 
> FFMPEG is required for this. 
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdemultimedia/CMakeLists.txt 1121489 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/CMakeLists.txt PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/.directory PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/AUTHORS PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/ChangeLog PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/README PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/filmstripfilter.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/filmstripfilter.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/histogram.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/ifilter.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/imagetypes.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/imagewriter.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/imagewriterfactory.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/moviedecoder.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/moviedecoder.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/pngwriter.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/pngwriter.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/videoframe.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/videothumbnailer.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/videothumbnailer.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/videothumbnailerc.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbnailer/videothumbnailerc.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/ffmpegthumbs/ffmpegthumbs.desktop PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/3818/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Dolphin with ffmpegthumbs
>   http://reviewboard.kde.org/r/3818/s/375/
> 
> 
> Thanks,
> 
> Andreas
> 
>