[vlc-devel] [PATCH 2/4] doc: add an option to build libvlc sample app from the documentation

Steve Lhomme robux4 at ycbcr.xyz
Fri Mar 5 07:18:15 UTC 2021


On 3/4/2021 3:34 PM, Alexandre Janniaux wrote:
> Hi,
> 
> On Thu, Mar 04, 2021 at 11:42:02AM +0100, Steve Lhomme wrote:
>> On 3/1/2021 11:15 AM, Marvin Scholz wrote:
>>>
>>>
>>> On 1 Mar 2021, at 7:41, Steve Lhomme wrote:
>>>
>>>> This is not a file we use in our build system. It's there for
>>>> documentation purpose, to guide users.
>>>> That doesn't mean there's only one way to build Qt project. We use
>>>> autotools to build our own Qt module. In the past I've used others
>>>> as well. I believe nowadays CMAKE is also a candidate. We'll
>>>> probably use Meson for it as well.
>>>> Pushing for qmake here when we already have all the autotools parts
>>>> ready and solid to do it make no sense to me.
>>>>
>>>>
>>>> Otherwise you should advocate to add MSVC projects for libvlc and
>>>> build windows samples with it since that's what a lot of people will
>>>> use.
>>>>
>>>
>>> I agree with what Alexandre already wrote, duplicating the build in
>>> autotools here seems incredibly awful choice
>>> for nearly no benefit at all… Just use qmake to build the sample
>>> project, like most users of it would do?
>>> I am not sure who would benefit from having another build for this in
>>> autotools, which can be painful to use
>>> properly with Qt, or nearly impossible even, on macOS. And additionally
>>> can easily get out of sync when
>>> someone is unaware of the other build system for it and makes changes
>>> only updating the autotools build for it…
>>>
>>> Your point about MSVC does not really make sense to me, as this is a
>>> completely different topic, really.
>>
>> Nope. this is exactly the confusion you are both making between building the
>> samples when building VLC and building the samples in an external context
>> with the purpose of integrating libvlc in an app.
> 
> No, I think you're confusing what a sample project is and what
> a test is. Using samples for the coverage of libvlc API is not
> correct.
> 
> Don't get me wrong, checking that the libvlc samples are correct
> _is_ a good move IMHO, like I already mentioned, but if you're
> not testing the buildsystem you're shipping in the documentation
> then the check makes no sense and it's quite easy to understand

I agree we can remove the dummy build project since we don't use it 
anyway. Other samples don't have this luxury. And as explained before we 
can't assume what build system people will use for their own system (ie 
we're not adding msvc+cmake+meson for win32 samples just because someone 
might use that).

> why having two buildsystems for the samples can actually lead to
> breakage of the shipped samples, especially if you develop using
> the non-shipped buildsystem. Just think missing dependency, source
> file not added, etc.
> 
> Despite of the different buildsystem for the new sample project,
> you can still compile them within the CI and even add them in the
> automake project.

This is a nightmare to manage, no thank you.

>> Building the samples with the VLC build is useful for the CI to make sure
>> changes to the API are also reflected in the sample code that users are
>> supposed to use.
>> It also helps tracking regression when doing a git bisect. Otherwise at
>> every step you need to apply/unapply this patch(set) in order to test the
>> git commit.
> 
> That's poor management excuses to be honest. If you need the

Thanks.

> latest version of the sample, you can just copy the project
> somewhere out of the version control system, and install libVLC

You probably never worked on Windows. Installing libvlc everytime you 
make a change in the code is taking forever, when in fact there's no 
reason to do it at all. The changes I propose allow developping the 
sample code without having to install anything. Just like making changes 
in VLC don't require it to install before testing the code. We have 
VLC_PLUGIN_PATH for that.

> while bisecting. If you need to sync the sample revision with
> the bisect, then you have nothing to change from now. If you

What ? This is exactly the thing I am missing and adding: having the 
samples build in sync with master. And what I repeatdly add to do 
manually in the last few years I worked on the samples.

> need to have different bisect between the sample and libvlc,

No idea who would ever want that.

> then you need two VCS, potentially apply a filter-branch to
> one of them to only keep doc/ commits (but it's not even
> needed, it's just to reduce the final complexity), and then
> you actually need to use external build system to be able to
> compile against the installed libvlc version.
> 
>> This is not meant to be used by external developers to integrate in their
>> own code.
> 
> I mostly criticize the way you do it, duplicating build system
> instead of relying on the existing supplied ones, not the intent.

Duplicating build system would be calling qmake,cmake,msvc,meson from 
autotools we are using today. You're still confused between internal and 
external use of the code.

>> There is no new complexity in autotools, it's using variables from the same
>> configure run.
> 
> You litterally add 100 lines of Makefile.am, a custom rule to
> build player.moc.cpp, add a strong dependency on the orders of
> SUBDIRS, and actually add a new AC_CONFIG_FILES which generates
> a whole new Makefile in the end. I fail to see how you don't
> add new complexity in autotools.

If you consider autotools complex sure. Calling qmake,cmake,msvc,meson 
from autotools will likely create *more* complexity. The MOC part here 
can probably be shared with the Qt modules we want to reduce duplicates. 
But as I said I don't care about that part so we can drop it.

>> But given I don't care too much about the Qt samples I can just do it for
>> the Win32 sample app which don't have a pretend makefile that we're
>> supposedly meant to use.
>>
>>>>> On 27 Feb 2021, at 20:15, Alexandre Janniaux
>>>>> <ajanni at videolabs.io> wrote:
>>>>>
>>>>> On Sat, Feb 27, 2021 at 06:20:49PM +0100, Steve Lhomme wrote:
>>>>>>> On 2/27/2021 3:56 PM, Alexandre Janniaux wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Sat, Feb 27, 2021 at 10:48:43AM +0100, Steve Lhomme wrote:
>>>>>>>> On 2021-02-26 18:16, Alexandre Janniaux wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> As mentioned privately, I'm not fond of adding yet another build
>>>>>>>>> system to build examples, especially since it won't even be testing
>>>>>>>>> the buildsystem final users are supposed to use.
>>>>>>>>
>>>>>>>> There is not "yet another build system" in this patch.
>>>>>>>
>>>>>>> Sorry, what? you litterally create doc/libvlc/Makefile.am
>>>>>>> typically to build QtGL and QtPlayer with new automake
>>>>>>> targets and even new automake rules whereas there is already
>>>>>>> a qmake-based build system.
>>>>>>
>>>>>> Our "build system" is autotools. I just use that.
>>>>>> Please point me to the "qmake-based build system" we already use.
>>>>>
>>>>> https://code.videolan.org/videolan/vlc/-/blob/master/doc/libvlc/QtGL/QtGl.pro
>>>>>
>>>>>
>>>>>>> With that, both can now get out-of-sync with regards to the
>>>>>>> other one, and one of them is even never used in the CI while
>>>>>>> the other would always be used. Better remove the other one
>>>>>>> like I mentioned then, but as I said:
>>>>>>>
>>>>>>>   - users will never build their application from the VLC build
>>>>>>>     directory.
>>>>>>>
>>>>>>>   - most Qt users will typically never use automake to build a
>>>>>>>     Qt application, so it seems like a bad target for
>>>>>>>     documentation.
>>>>>>>
>>>>>>>   - in addition, it means that you need to build libvlc in order
>>>>>>>     to build the examples, which is quite inconvenient since you
>>>>>>>     could be able to ship the examples with a packaged libvlc.
>>>>>>>
>>>>>>>> The final users are free to use whatever they want
>>>>>>>> and that's a good reason
>>>>>>>> not to orientate them in any favored direction.
>>>>>>>
>>>>>>>   - and I completely agree with this point right above, which is
>>>>>>>     why using automake within the same autoconf package is a bad
>>>>>>>     idea, since it will never be the same environment for other
>>>>>>>     buildsystem, by-pass potential dependency detection systems
>>>>>>>     and thus is quite opinionated. But more on this in next
>>>>>>>     paragraph.
>>>>>>>
>>>>>>>>
>>>>>>>>> I'd prefer that we typically use make install to a given prefix and
>>>>>>>>> then use qmake/foomake/etc with the correct PKG_CONFIG_PATH. The
>>>>>>>>> alternative is to use only the automake buildsystem but to be fair,
>>>>>>>>
>>>>>>>> Last time I proposed a similar patch, Rémi advised
>>>>>>>> to use "noinst_" instead
>>>>>>>> of "bin_" prefix. And IMO it makes sense not to
>>>>>>>> install sample code. The
>>>>>>>> solution you propose is the opposite, it requires to
>>>>>>>> install libvlc before
>>>>>>>> building the samples. Something that is not needed
>>>>>>>> by us nor the end user
>>>>>>>> who will need to repackage the build anyway.
>>>>>>>
>>>>>>> There's a huge difference between "do not install the examples"
>>>>>>> (which makes perfect sense because users generally don't need
>>>>>>> them on their systems) and "do not install libvlc".
>>>>>>>
>>>>>>> Typically, if you don't install libvlc, but use the mock plugins
>>>>>>> in the examples, it might work with the automake buildsystem you
>>>>>>> wrote but won't work with a packaged VLC and an external build
>>>>>>> system, because mock plugin is not installed.
>>>>>>>
>>>>>>> So I really fail to see how it's the opposite.
>>>>>>>
>>>>>>> In addition, if libvlc needs to be repackaged (which imho is
>>>>>>> already a flaw in our buildsystem since it bypass the make
>>>>>>> install behaviour and is another layer above the buildsystem
>>>>>>> which actually mimic the layer below instead of using it), then
>>>>>>> we just need to actually package libvlc in the CI before using
>>>>>>> it in the examples.
>>>>>>>
>>>>>>>>> what are the odds that final users are building their application
>>>>>>>>> inside libvlc buildsystem without having an installed package?
>>>>>>>>
>>>>>>>> Maybe it's customary on Linux (is there a separate
>>>>>>>> libvlc package on Linux
>>>>>>>> distros ?) but on Windows or Mac or Android or iOS
>>>>>>>> they will need to package
>>>>>>>> libvlc by their own means, with their build system.
>>>>>>>> So the way we build it
>>>>>>>> for ourself will not have much impact.
>>>>>>>
>>>>>>> I don't think many people wants to be compiling libvlc
>>>>>>> themselves, even on Windows, and especially on Windows.
>>>>>>>
>>>>>>> But anyway say that this is true. Why use libvlc from
>>>>>>> the build directory instead of using libvlc from the
>>>>>>> packaged directory then?
>>>>>>>
>>>>>>> Then, yes, we have tools to detect dependencies much like
>>>>>>> any other platforms. We don't typically use additional
>>>>>>> layer of dependency management like Conan but the user
>>>>>>> might use that to provide libvlc support. By using external
>>>>>>> build system (and actually any external buildsystem), we
>>>>>>> show the user how it can use it to build with libvlc with
>>>>>>> minor tweaking to the dependency layer, be it cmake, meson,
>>>>>>> qmake, conan or anything other used on Windows. How is using
>>>>>>> uninstalled libtool library which almost none of the above
>>>>>>> buildsystem understand is less opinionated than using the
>>>>>>> idiomatic buildsystem used within the community the examples
>>>>>>> are made for?
>>>>>>>
>>>>>>> Just so this message is clear, I'd be happy that we test the
>>>>>>> examples in the CI and I'm glad that you initiated work
>>>>>>> regarding that. But honestly even the previous buildsystem
>>>>>>> was broken last time I used it and I had to fix it, I don't
>>>>>>> want another maintenance burden there and especially if it
>>>>>>> gives no additional value to the enduser.
>>>>>>>
>>>>>>> The --enable-doc-samples can still be in place in configure
>>>>>>> with much less code and without duplicating the current work.
>>>>>>>
>>>>>>> Regards,
>>>>>>> --
>>>>>>> Alexandre Janniaux
>>>>>>> Videolabs
>>>>>>> _______________________________________________
>>>>>>> vlc-devel mailing list
>>>>>>> To unsubscribe or modify your subscription options:
>>>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> vlc-devel mailing list
>>>>>> To unsubscribe or modify your subscription options:
>>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>> _______________________________________________
>>>>> vlc-devel mailing list
>>>>> To unsubscribe or modify your subscription options:
>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>> _______________________________________________
>>>> vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 




More information about the vlc-devel mailing list