[KPhotoAlbum] new version of the videopatch
Johannes Zarl-Zierl
johannes at zarl-zierl.at
Fri Sep 15 21:47:30 BST 2017
Hi Andreas,
I've noticed another small issue in your code:
You use QString::split() and work around possible empty fields. There's also
the SplitBehavior parameter [1].
[1] https://doc.qt.io/qt-5/qstring.html#SplitBehavior-enum
Also, I've taken a closer look at the problem you're trying to solve (sorry
for not doing so earlier, but taking a quick look at the formalities of a
patch takes quite a lot less time than really understanding it *g*).
For the length extraction, you replace:
ffprobe -v error -select_streams v:0 -show_entries stream=duration -of
default=noprint_wrappers=1:nokey=1 [filename]
with:
ffprobe -hide_banner [filename]
plus some text parsing.
I've got two issues with this:
1. Parsing text in the default format of ffprobe is likely to break some time
in the future. Setting the output format explicitly should reduce the likely-
hood of that happening.
2. Maybe it's enough to use the container duration instead of the stream
duration:
ffprobe -v error -show_entries format=duration -of
default=noprint_wrappers=1:nokey=1 [filename]
It works for the "surf_video.flv" error clip that you provided earlier. If it
works for all your clips, we can stick with the simple parsing we had earlier.
On Freitag, 15. September 2017 19:36:38 CEST Andreas Schleth wrote:
> > 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.
>
> Done - there were a few remains of markShort ...
Thanks.
> > 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().
>
> 1 dDebug exchanged for qWarning. I'd like to keep the commented out
> qDebugs in if I encounter some more strange or broken videos
Better than commenting them out is the following:
1. Add (I've done this already in git master):
#if DEBUG_IMAGEMANAGER
# define Debug qDebug
#else
# define Debug if(0) qDebug
#endif
2. Use "Debug()" instead of "qDebug()"
→ The advantage is reduced bit-rot. E.g. if somebody renames a variable using
standard refactoring tools, active code is changed accordingly, but comments
are not.
> Johannes, please feel free to improve on my patchy programming skills
> and change the patch as needed.
> I know a bit about algorithms and such, but unfortunately nearly nothing
> about C++. All I did, was some imitation of existing code and guessing
> my way around.
As the saying goes:
Fix a patch for somebody and he will code for a day. Teach a man how to fix a
patch and his patches will be plentiful for evermore... ;-)
I've again run out of KPA time. I'll try to dedicate a little bit more dev-
time on KPA in the (hopefully) near future.
Cheers,
Johannes
P.S.: @Robert: If you are reading this: I've not forgotten your patchset, I
simply had no time to look further into it :|
More information about the Kphotoalbum
mailing list