hi, how do projects usually manage binary incompatibilities in different v4l-utils minor releases?
we (qt) recently received this patch: https://codereview.qt-project.org/#/c/251520/1//ALL
and we don't want to break older platforms
johanhelsing: this change happened ~3 years ago:
commit dad4a1cc932dfc235078d7d04b7c271eb25ca5b7
Author: Sakari Ailus <sakari.ailus@linux.intel.com>
Date:   Thu Mar 17 16:27:09 2016 +0200
    libmediactl: Drop length argument from media_get_entity_by_name()
in thesis, we shouldn't be breaking kABI for libraries, but afaikt libmediactl was never considered an "exported" library
mchehab: yeah, but we still have customers on really old platforms, and we want them to be able to upgrade qt
(it doesn't install by make install)
sailus and pinchartl are the ones that are working with this library
(IMHO, it has some issues and need to be rewritten in order to support newer media controller ioctls)
the problem is that, if one changes it right now to return to its old behavior, it will brake stuff as well
s/brake;break;
mchehab: so we probably shouldn't have used it then, and just do the ioctls ourselves instead?
if you have a suggestion about how to fix it in a proper way, feel free to submit a patch, but I can't see how to avoid the issue, as the kABI breakage is not recent
i was hoping for someting like a define for the api version, and then I could just have done an ifdef in my code
works for me
qt would have to be recompiled when v4l is updated, but I think that's fine for most use cases
yeah, it should be fine to recompile qt if v4l-utils is updated
there are macros there already exposing its version
internally
(and even a stringfyed one: V4L_UTILS_VERSION)
it shouldn't be hard to add a .h file there containing the values of such macros
AC_DEFINE_UNQUOTED([V4L_UTILS_VERSION], ["$PACKAGE_VERSION"], [v4l-utils version string])
MAJOR=`echo "$PACKAGE_VERSION" | perl -ne 'print $1 if (m/^(\d+)\.(\d+)\.(\d+)/)'`
MINOR=`echo "$PACKAGE_VERSION" | perl -ne 'print $2 if (m/^(\d+)\.(\d+)\.(\d+)/)'`
PATCH=`echo "$PACKAGE_VERSION" | perl -ne 'print $3 if (m/^(\d+)\.(\d+)\.(\d+)/)'
you could actually check the version already... those macros are at:
lib/include/libdvbv5/libdvb-version.h.in
$ more lib/include/libdvbv5/libdvb-version.h.in
#define LIBDVBV5_VERSION_MAJOR	@MAJOR@
#define LIBDVBV5_VERSION_MINOR	@MINOR@
#define LIBDVBV5_VERSION_PATCH	@PATCH@
#define LIBDVBV5_VERSION	"@PACKAGE_VERSION@"
if you include this header, you could check the v4l-utils package version
the problem is that this header file may not be included on all distros, as it may be packaged on a different -dev package
yet, it should be easy to add a similar set of macros to libmediactl header file
ah, that's a problem then :/
johanhelsing: The libv4l2subdev and libmediactl libraries are under utils (rather than lib) and not exported for just that reason: we weren't all that sure about the API back then.
yep, it will add another packaging dependency for qt
I remember pinchartl did some work years ago to support virtual devices but that didn't finish in a meaningful way AFAIR.
sailus: making it compatible with G_TOPOLOGY ioctl would be *really* painful and hacky
IMO, the best would be to use a different API that would be more keen with that
anyway, we now have a problem: some qt work depends on it
sailus: I see, I wasn't aware of that when I wrote the thing. I guess we should try to move away from those private apis, eventually. In the meantime I'll try to see if if some SFINAE magic can solve
johanhelsing: is it the Qt core libraries itself or just some qt application?
mchehab: very limited scope so far
ok, but inside qt core or as another project?
it's in a qtbase platform plugin for devices with vsp2
johanhelsing: API compatibility hasn't really been thought about. I don't see a reason to start providing that in the future, given that there's been just a single patch over the years changing things.
so yes, qtcore, but not normally built
I wonder what pinchartl thinks.
johanhelsing: ok... then perhaps you could just copy the files inside the plugin
mchehab: maybe? I guess they're just convenience for ioctls?
(not ideal, but it would avoid further issues, specially as we'll need to touch at the API in order to make it good enough for the current media controller features)
they do more than just exposing the ioctls
are you using it to set pipelines or just to get the topology?
hmm... it's a long time ago, let me check
we do stuff like media_setup_link, which I guess would be setting pipelines?
yes
so, I guess it would be better to stick with the library
IMHO, I would copy the files to the qt plugin and avoid adding more stuff using it
if you ever need to add more stuff, please ping us first... we need to redesign the library in order to solve some things, like mapping entities per function(s), supporting properties and  interfaces (with the currently library design won't be able to support without adding hacks, due to some of the API assumptions)
you may, instead use that *.h file I pointed you...
or send us a patch adding defines at libmediactl.h storing v4l-utils version
but the problem with the latter approach is that distros will need to backport such patch if they have a v4l-utils version after March, 2016
the header we already have was added on Oct, 2016:
commit ab61415b004a36ec0bc5c70a1012898758625793
Author: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Date:   Tue Oct 4 08:18:43 2016 -0300
    libdvbv5: add a header with the library version
(v4l-utils-1.10.0-291-gab61415b004a)
hmm...
$ git describe dad4a1cc932dfc235078d7d04b7c271eb25ca5b7
v4l-utils-1.10.0-90-gdad4a1cc932d
both the kAPI breakage and the libdvbv5 version header were added at the same v4l-utils version
so, the only issue you have is to ensure that distros using the vsp2 plugin should package this header together with mediactl.h
I suspect that this should be easy, as the scope here is limited
(probably, you need to look only on embedded distros like Yocto, OpenWrt, ...) in order to check that
mchehab: I think it's only that one function, and I think I can solve it for now by writing a function that takes media_get_entity_by_name as an argument and write two overloads for it based on its signature. It's absolutely not pretty, but it should work
alos, thanks a lot for the suggestions, I'll create a task for it in our bug tracker, and it might be done eventually, but for now I don't have the hardware set up to test, so I'd like my changes to be as non-intrusive as possible
I'll also put in some warnings so people don't start using it in other parts of Qt
ok, good
mchehab: ugly hack here if you're interested... I think it should work: https://codereview.qt-project.org/#/c/251520/4//ALL
sounds interesting... I suspect that it should work if libmediactl is compiled with c++
not sure if it would work if built with standard C
hverkuil: you around?
ezequielg: yup
hverkuil: v4l2_m2m_next_src_buf and friends are all returning void *
it seems it would be better to return the real type, vb2_v4l2_buffer.
but at the same time, maybe it's intentional, to allow the type to change?
i am having mixed thoughts.
currently, some drivers use vb2_v4l2_buffer, some vb2_buffer as the return type. of course, both works.
but it feels akward to have a void * here.
I agree. It should return a struct vb2_v4l2_buffer pointer.
I already started doing the work.
great! I hate void *
but many drivers need to be fixed, and it's a pita.
annoying to touch drivers i can't test.
guess, we'll have to review it carefully.
It's pretty straightforward, I don't expect any problems.
well, the devil is in the detail.