FFMpeg plugin cleanup
Needs ReviewPublic

Authored by blaze on Apr 11 2018, 2:48 PM.

Details

Reviewers
lesliezhai
aacid
Summary

Remove legacy code. This code is targeting APIs that do not exist for years, making everything look cluttered, practically preventing further improvements to this plugin.

Diff Detail

Repository
R467 K3b
Lint
Lint Skipped
Unit
Unit Tests Skipped
blaze requested review of this revision.Apr 11 2018, 2:48 PM
blaze created this revision.
blaze created this object with visibility "All Users".
aacid added a comment.Apr 12 2018, 5:27 PM

Don't really have a say on k3b, but sounds sensible

blaze added a reviewer: pino.Apr 26 2018, 6:44 AM
pino resigned from this revision.Apr 26 2018, 8:39 PM
pino removed a reviewer: pino.
ltoscano changed the visibility from "All Users" to "Public (No Login Required)".Apr 27 2018, 3:07 PM
bruns added a subscriber: bruns.Apr 27 2018, 3:31 PM

Are you planning to do more cleanups? There is quite some more stuff, e.g. in https://phabricator.kde.org/source/k3b/browse/master/CMakeLists.txt

I think a proper version check for libavcodec/libavformat should be added to the toplevel CMakeLists.txt first, as you are relying on it here.

blaze added a comment.Apr 27 2018, 3:56 PM

Are you planning to do more cleanups? There is quite some more stuff, e.g. in https://phabricator.kde.org/source/k3b/browse/master/CMakeLists.txt

I think a proper version check for libavcodec/libavformat should be added to the toplevel CMakeLists.txt first, as you are relying on it here.

I was planning to port the code to the current API and then reflect all those changes at once inside CMake files (one more patch).
But was it me who added libavformat dependency here? I don't think so.

bruns added a comment.Apr 27 2018, 4:00 PM

Are you planning to do more cleanups? There is quite some more stuff, e.g. in https://phabricator.kde.org/source/k3b/browse/master/CMakeLists.txt

I think a proper version check for libavcodec/libavformat should be added to the toplevel CMakeLists.txt first, as you are relying on it here.

I was planning to port the code to the current API and then reflect all those changes at once inside CMake files (one more patch).
But was it me who added libavformat dependency here? I don't think so.

Before your changes, the code works for quite old versions of libavformat, after your change it requires libavformat > 56 (?), so you are changing the requirements.

blaze added a comment.Apr 27 2018, 4:05 PM

Before your changes, the code works for quite old versions of libavformat, after your change it requires libavformat > 56 (?), so you are changing the requirements.

Right. That's a better explanation. I'll see what I can do here.

adridg added a subscriber: adridg.Apr 27 2018, 7:34 PM

What you could do, is to ask on the distributions@k.o list, to find out who ships what. FreeBSD, for instance, has /usr/local/lib/libavformat.so.57.83.100 so that looks like it would be ok. Debian stable, on the other hand ..

blaze added a comment.Apr 27 2018, 9:19 PM

What you could do, is to ask on the distributions@k.o list, to find out who ships what. FreeBSD, for instance, has /usr/local/lib/libavformat.so.57.83.100 so that looks like it would be ok. Debian stable, on the other hand ..

Sigh. Stretch will never get this code anyway.