After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 797165 - Wrong include directives for gstreamer-base headers
Wrong include directives for gstreamer-base headers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: documentation
git master
Other Linux
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-09-18 13:56 UTC by Linus Svensson
Modified: 2018-09-19 14:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to correct documentation (1.03 KB, patch)
2018-09-18 13:56 UTC, Linus Svensson
none Details | Review
Change to include gst/base/base.h (3.96 KB, patch)
2018-09-19 12:14 UTC, Linus Svensson
committed Details | Review

Description Linus Svensson 2018-09-18 13:56:36 UTC
Created attachment 373689 [details] [review]
Patch to correct documentation

Two include directives for headers in gstreamer-base is wrong.

gstadapter.h: It was just gst/libs/adapter.h, which doesn't work at all
gstaggregator.h: It was gst/libs/base.h, which works but all other include directives used the individual .h files.

It looks like base-prelude.h never is included since most places don't include gst/libs/base.h, but rather whatever header file is needed directly.

Anyway I made a patch that uniforms the documentation.
Comment 1 Sebastian Dröge (slomo) 2018-09-18 14:08:11 UTC
Comment on attachment 373689 [details] [review]
Patch to correct documentation

It would be better to update them all to the single-include headers instead
Comment 2 Tim-Philipp Müller 2018-09-18 14:11:56 UTC
> gstadapter.h: It was just gst/libs/adapter.h, which doesn't work at all

Oops, thanks.

> gstaggregator.h: It was gst/libs/base.h, which works but all other include
> directives used the individual .h files.
> 
> It looks like base-prelude.h never is included since most places don't
> include gst/libs/base.h, but rather whatever header file is needed directly.

I think maybe we should change the documentation to include he 'global' header instead, which would also include -prelude.h then, as not doing so may soon become a problem (however, I've also added the prelude include to many header files like gstaggregator.h IIRC since we need to maintain backwards compat to some extent).
Comment 3 Linus Svensson 2018-09-19 12:14:19 UTC
Created attachment 373704 [details] [review]
Change to include gst/base/base.h
Comment 4 Sebastian Dröge (slomo) 2018-09-19 12:22:13 UTC
Comment on attachment 373704 [details] [review]
Change to include gst/base/base.h

Looks good to me, Tim?

Should probably do the same for everything in sections.txt :)
Comment 5 Linus Svensson 2018-09-19 12:24:20 UTC
I didn't look through everything. Looked like gstcontroller already worked this way, so I didn't do a complete overview. :(
Comment 6 Tim-Philipp Müller 2018-09-19 13:27:05 UTC
Looks good to me, thanks! :)
Comment 7 Tim-Philipp Müller 2018-09-19 13:57:30 UTC
commit 2e6089f0c5c6024d026914596c4492d0fc3f6730 (HEAD -> master)
Author: Linus Svensson <linussn@axis.com>
Date:   Tue Sep 18 15:44:24 2018 +0200

    docs: Update include directive for gstreamer-base components
    
    Change to always include gst/libs/base.h in order to also
    include base-prelude.h, but also because it's the right
    thing for people to include anyway.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797165
Comment 8 Tim-Philipp Müller 2018-09-19 14:09:02 UTC
commit 4aef0fca96b03ea58bf3de9d6fd234db39b3a3c2
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Wed Sep 19 15:07:36 2018 +0100

    docs: libs: move all includes to canonical single header includes
    
    And fix up bogus libs/ prefix for controller lib includes.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797165