Bug 142778 – [EFL] Add OpenWebRTC in jhbuild

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Philippe Normand]

.

[Philippe Normand]

Created attachment 248835 [details]
patch

[Csaba Osztrogonác]

Please wait with landing until I install new dependencies 
on the bots and check if everything will be OK.

[Gyuyoung Kim]

Thank you for supporting openWebRTC for EFL port as well. I will install dependencies pkgs within a couple of hours.

[Csaba Osztrogonác]

I get the same error as the EFL EWS, but I have no idea what is it ...

jhbuild build: branch for gst-plugins-openwebrtc is not correct, check the moduleset file.
Running jhbuild-wrapper build failed.

[Csaba Osztrogonác]

I think I got it, github.com has tarball repository type for EFL and git for GTK.

[Csaba Osztrogonác]

(In reply to comment #4)
> I get the same error as the EFL EWS, but I have no idea what is it ...
> 
> jhbuild build: branch for gst-plugins-openwebrtc is not correct, check the
> moduleset file.
> Running jhbuild-wrapper build failed.

(In reply to comment #5)
> I think I got it, github.com has tarball repository type for EFL and git for
> GTK.

I checked, adding another github repository with git type
for the new modules fixes this issue. I'm checking the build now.

[Csaba Osztrogonác]

one more error later:

configure: error: Package requirements (gstreamer-1.0 >= 1.4 gstreamer-rtp-1.0 >= 1.4 gstreamer-video-1.0 >= 1.4 gstreamer-app-1.0 >= 1.4) were not met:

Requested 'gstreamer-1.0 >= 1.4' but version of GStreamer is 1.2.1
Requested 'gstreamer-rtp-1.0 >= 1.4' but version of GStreamer RTP Library is 1.2.1
Requested 'gstreamer-video-1.0 >= 1.4' but version of GStreamer Video Library is 1.2.1
Requested 'gstreamer-app-1.0 >= 1.4' but version of GStreamer Application Library is 1.2.1


Does it mean that we should bump gstreamer version too?

[Philippe Normand]

(In reply to comment #7)
> one more error later:
> 
> configure: error: Package requirements (gstreamer-1.0 >= 1.4
> gstreamer-rtp-1.0 >= 1.4 gstreamer-video-1.0 >= 1.4 gstreamer-app-1.0 >=
> 1.4) were not met:
> 
> Requested 'gstreamer-1.0 >= 1.4' but version of GStreamer is 1.2.1
> Requested 'gstreamer-rtp-1.0 >= 1.4' but version of GStreamer RTP Library is
> 1.2.1
> Requested 'gstreamer-video-1.0 >= 1.4' but version of GStreamer Video
> Library is 1.2.1
> Requested 'gstreamer-app-1.0 >= 1.4' but version of GStreamer Application
> Library is 1.2.1
> 
> 
> Does it mean that we should bump gstreamer version too?

Yep

[Csaba Osztrogonác]

(In reply to comment #8)
> (In reply to comment #7)
> > one more error later:
> > 
> > configure: error: Package requirements (gstreamer-1.0 >= 1.4
> > gstreamer-rtp-1.0 >= 1.4 gstreamer-video-1.0 >= 1.4 gstreamer-app-1.0 >=
> > 1.4) were not met:
> > 
> > Requested 'gstreamer-1.0 >= 1.4' but version of GStreamer is 1.2.1
> > Requested 'gstreamer-rtp-1.0 >= 1.4' but version of GStreamer RTP Library is
> > 1.2.1
> > Requested 'gstreamer-video-1.0 >= 1.4' but version of GStreamer Video
> > Library is 1.2.1
> > Requested 'gstreamer-app-1.0 >= 1.4' but version of GStreamer Application
> > Library is 1.2.1
> > 
> > 
> > Does it mean that we should bump gstreamer version too?
> 
> Yep

Let's do it first ( filed a new bug report - bug142782 )
and then check this bug.

[Gyuyoung Kim]

I install libjson-glib-dev liborc-0.4-dev libv4l-dev in EFL EWS and buildbot now. Please test this patch after bumping gstreamer again.

[Csaba Osztrogonác]

I got the following builf error during jhbuild on our ARM bots:

user_recv_thread.c: In function 'recv_thread_init':
user_recv_thread.c:1164:99: error: ISO C prohibits argument conversion to union type [-Werror=pedantic]
     if (bind(SCTP_BASE_VAR(userspace_rawsctp), (const struct sockaddr *)&addr_ipv4, sizeof(struct sockaddr_in)) < 0) {
                                                                                                   ^
user_recv_thread.c:1226:99: error: ISO C prohibits argument conversion to union type [-Werror=pedantic]
     if (bind(SCTP_BASE_VAR(userspace_udpsctp), (const struct sockaddr *)&addr_ipv4, sizeof(struct sockaddr_in)) < 0) {
                                                                                                   ^
user_recv_thread.c:1300:101: error: ISO C prohibits argument conversion to union type [-Werror=pedantic]
      if (bind(SCTP_BASE_VAR(userspace_rawsctp6), (const struct sockaddr *)&addr_ipv6, sizeof(struct sockaddr_in6)) < 0) {
                                                                                                     ^
user_recv_thread.c:1372:100: error: ISO C prohibits argument conversion to union type [-Werror=pedantic]
     if (bind(SCTP_BASE_VAR(userspace_udpsctp6), (const struct sockaddr *)&addr_ipv6, sizeof(struct sockaddr_in6)) < 0) {
                                                                                                    ^
cc1: all warnings being treated as errors
make[1]: *** [libusrsctp_la-user_recv_thread.lo] Error 1
make[1]: *** Waiting for unfinished jobs....

I'll investigate it.

[Csaba Osztrogonác]

Comment on attachment 248835 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248835&action=review

> Tools/efl/jhbuild.modules:306
> +  <autotools id="libusrsctp" autogen-sh="./bootstrap; ./configure">
> +    <branch repo="sctp-refimpl-google" module="trunk/KERN/usrsctp/" revision="r9168"/>
> +  </autotools>

Unfortunately jhbuild ignores the revision and always checks out the top of trunk. It isn't safe at all.

[Csaba Osztrogonác]

(In reply to comment #11)
> I got the following builf error during jhbuild on our ARM bots:
> 
> user_recv_thread.c: In function 'recv_thread_init':
> user_recv_thread.c:1164:99: error: ISO C prohibits argument conversion to
> union type [-Werror=pedantic]
>      if (bind(SCTP_BASE_VAR(userspace_rawsctp), (const struct sockaddr
> *)&addr_ipv4, sizeof(struct sockaddr_in)) < 0) {
>                                                                             
> ^
> user_recv_thread.c:1226:99: error: ISO C prohibits argument conversion to
> union type [-Werror=pedantic]
>      if (bind(SCTP_BASE_VAR(userspace_udpsctp), (const struct sockaddr
> *)&addr_ipv4, sizeof(struct sockaddr_in)) < 0) {
>                                                                             
> ^
> user_recv_thread.c:1300:101: error: ISO C prohibits argument conversion to
> union type [-Werror=pedantic]
>       if (bind(SCTP_BASE_VAR(userspace_rawsctp6), (const struct sockaddr
> *)&addr_ipv6, sizeof(struct sockaddr_in6)) < 0) {
>                                                                             
> ^
> user_recv_thread.c:1372:100: error: ISO C prohibits argument conversion to
> union type [-Werror=pedantic]
>      if (bind(SCTP_BASE_VAR(userspace_udpsctp6), (const struct sockaddr
> *)&addr_ipv6, sizeof(struct sockaddr_in6)) < 0) {
>                                                                             
> ^
> cc1: all warnings being treated as errors
> make[1]: *** [libusrsctp_la-user_recv_thread.lo] Error 1
> make[1]: *** Waiting for unfinished jobs....

libusrsctp/configure:
...

linux*)
  CFLAGS="$CFLAGS -pedantic -Wall -Werror -pthread -D_GNU_SOURCE"
  LIBCFLAGS="$LIBCFLAGS  -D__Userspace_os_Linux"
  ;;

...

Unfortunately it isn't possible to supress -pedantic with adding CFLAGS
to the configure. And it isn't possible to patch the source, because it
is an SVN source and jhbuild can patch only tarballs.

Do you think if we should create a fork on github and remove pedantic there?

[Philippe Normand]

(In reply to comment #12)
> Comment on attachment 248835 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248835&action=review
> 
> > Tools/efl/jhbuild.modules:306
> > +  <autotools id="libusrsctp" autogen-sh="./bootstrap; ./configure">
> > +    <branch repo="sctp-refimpl-google" module="trunk/KERN/usrsctp/" revision="r9168"/>
> > +  </autotools>
> 
> Unfortunately jhbuild ignores the revision and always checks out the top of
> trunk. It isn't safe at all.

Wow, wtf... The revision attr seems used to indicate a *branch* name if I read the JHBuild code correctly. This is wrong in so many undescriptive ways.

[Philippe Normand]

(In reply to comment #14)
> (In reply to comment #12)
> > Comment on attachment 248835 [details]
> > patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=248835&action=review
> > 
> > > Tools/efl/jhbuild.modules:306
> > > +  <autotools id="libusrsctp" autogen-sh="./bootstrap; ./configure">
> > > +    <branch repo="sctp-refimpl-google" module="trunk/KERN/usrsctp/" revision="r9168"/>
> > > +  </autotools>
> > 
> > Unfortunately jhbuild ignores the revision and always checks out the top of
> > trunk. It isn't safe at all.
> 
> Wow, wtf... The revision attr seems used to indicate a *branch* name if I
> read the JHBuild code correctly. This is wrong in so many undescriptive ways.

Ok I take this back :) Pfeww.

Can you try with revision="9168" ? That should work according to https://git.gnome.org/browse/jhbuild/tree/jhbuild/versioncontrol/svn.py#n234

[Csaba Osztrogonác]

(In reply to comment #15)
> Can you try with revision="9168" ? That should work according to
> https://git.gnome.org/browse/jhbuild/tree/jhbuild/versioncontrol/svn.py#n234

Will check.

[Csaba Osztrogonác]

(In reply to comment #16)
> (In reply to comment #15)
> > Can you try with revision="9168" ? That should work according to
> > https://git.gnome.org/browse/jhbuild/tree/jhbuild/versioncontrol/svn.py#n234
> 
> Will check.

s/r9168/9168 works ;) But unfortunaltely the pedantic issue is still valid,
so I started to create a fork on github to be able suppress this error.

[Csaba Osztrogonác]

Comment on attachment 248835 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248835&action=review

> Tools/efl/jhbuild.modules:313
> +   <branch repo="github.com" module="EricssonResearch/openwebrtc-gst-plugins.git" checkoutdir="gst-plugins-openwebrtc" tag="master"/>

Relying on master isn't safe at all.

[Philippe Normand]

(In reply to comment #18)
> Comment on attachment 248835 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248835&action=review
> 
> > Tools/efl/jhbuild.modules:313
> > +   <branch repo="github.com" module="EricssonResearch/openwebrtc-gst-plugins.git" checkoutdir="gst-plugins-openwebrtc" tag="master"/>
> 
> Relying on master isn't safe at all.

Take current ToT then.

[Csaba Osztrogonác]

Created attachment 248929 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 248929 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248929&action=review

I used tarballs instead of git everywhere, because qemu doesn't like git which would break our ARM bots.

> Tools/efl/jhbuild.modules:305
> +  <autotools id="libusrsctp" autogen-sh="./bootstrap; ./configure">
> +    <branch repo="github.com" module="ossy-szeged/sctp-refimpl/archive/libusrsctp-r9168.tar.gz" version="r9168" checkoutdir="libusrsctp"
> +            hash="sha256:c1947e6a67fdc0d12f9f078265141125d0df019c7189d60e24f2331f4b6aaec2"
> +            md5sum="468386780c94370b1fb9b446ae3011ad" size="629523">
> +    </branch>
> +  </autotools>

I imported https://code.google.com/p/sctp-refimpl to github, because google code will be closed
and we need to disable -pedantic build due to https://bugs.webkit.org/show_bug.cgi?id=142778#c11 .

After r9168 I removed everything but libusrsctp and then disabled pedantic, nothing else changed.

> Tools/efl/jhbuild.modules:316
> +  <autotools id="gst-plugins-openwebrtc" supports-parallel-builds="no" autogen-sh="./autogen.sh; ./configure">
> +    <dependencies>
> +      <dep package="gst-plugins-base"/>
> +      <dep package="libusrsctp"/>
> +    </dependencies>
> +   <branch repo="github.com" module="ossy-szeged/openwebrtc-gst-plugins/archive/gst-plugins-openwebrtc-20150317.tar.gz" checkoutdir="gst-plugins-openwebrtc" version="gst-plugins-openwebrtc-20150317"
> +           hash="sha256:4ddb1ab20d08c323dcbcbb17f7f29994b51d9bf39669f5f7c21381de9926d189"
> +           md5sum="298f7d2c8f2e8f2999559df70c14074c" size="92809">
> +   </branch>
> +  </autotools>

It is a simple fork of the original repository - https://github.com/EricssonResearch/openwebrtc-gst-plugins ,
I only added the gst-plugins-openwebrtc-20150317 tag to be able stick on a revision in jhbuild.

[Philippe Normand]

(In reply to comment #21)
> 
> > Tools/efl/jhbuild.modules:316
> > +  <autotools id="gst-plugins-openwebrtc" supports-parallel-builds="no" autogen-sh="./autogen.sh; ./configure">
> > +    <dependencies>
> > +      <dep package="gst-plugins-base"/>
> > +      <dep package="libusrsctp"/>
> > +    </dependencies>
> > +   <branch repo="github.com" module="ossy-szeged/openwebrtc-gst-plugins/archive/gst-plugins-openwebrtc-20150317.tar.gz" checkoutdir="gst-plugins-openwebrtc" version="gst-plugins-openwebrtc-20150317"
> > +           hash="sha256:4ddb1ab20d08c323dcbcbb17f7f29994b51d9bf39669f5f7c21381de9926d189"
> > +           md5sum="298f7d2c8f2e8f2999559df70c14074c" size="92809">
> > +   </branch>
> > +  </autotools>
> 
> It is a simple fork of the original repository -
> https://github.com/EricssonResearch/openwebrtc-gst-plugins ,
> I only added the gst-plugins-openwebrtc-20150317 tag to be able stick on a
> revision in jhbuild.

Isn't it possible to specify a git commit hash?

[Csaba Osztrogonác]

(In reply to comment #22)
> > It is a simple fork of the original repository -
> > https://github.com/EricssonResearch/openwebrtc-gst-plugins ,
> > I only added the gst-plugins-openwebrtc-20150317 tag to be able stick on a
> > revision in jhbuild.
> 
> Isn't it possible to specify a git commit hash?

I didn't know if it is possible to download a given hash as tar.gz,
but it seems it works. Let me update this part of the patch.

[Csaba Osztrogonác]

The build still fails on the EFL EWS:

bindings/java/Makefile.am:41: error: HAVE_INTROSPECTION does not appear in AM_CONDITIONAL
bindings/java/Makefile.am: installing './depcomp'
owr/Makefile.am:84: error: HAVE_INTROSPECTION does not appear in AM_CONDITIONAL
autoreconf: automake failed with exit status: 1
autogen.sh failed

Do we need gobject-introspection package too?
(I have installed it locally, so I didn't notice this fail.)

[Philippe Normand]

I don't think it is strictly needed, try adding --enable-instrospection=no to autogenargs.

In WebKit we plan to use only the C/GObject API, afaik.

[Csaba Osztrogonác]

Created attachment 248931 [details]
Patch

gst-plugins-openwebrtc source changed back to ToT of EricssonResearch, added --disable-introspection to try to make EFL EWS happy.

[Csaba Osztrogonác]

(In reply to comment #26)
> Created attachment 248931 [details]
> Patch
> 
> gst-plugins-openwebrtc source changed back to ToT of EricssonResearch, added
> --disable-introspection to try to make EFL EWS happy.

Still same error on the EFL EWS. I have no idea what is it and only 
Gyuyoung has access to the bot, so I won't be able to debug this issue.

[Philippe Normand]

Comment on attachment 248931 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248931&action=review

> Tools/efl/jhbuild.modules:328
> +  <autotools id="openwebrtc" autogenargs="--enable-bridge=no --disable-introspection">

I don't see any --disable-introspection option in configure --help.

[Csaba Osztrogonác]

(In reply to comment #26)
> Created attachment 248931 [details]
> Patch
> 
> gst-plugins-openwebrtc source changed back to ToT of EricssonResearch, added
> --disable-introspection to try to make EFL EWS happy.

With this patch I got this error:

cp -f `find .. -name Owr-0.1.gir.h || echo "Owr-0.1.gir.h"` .
  CC       owr_utils.lo
cp: missing destination file operand after ‘.’

( I got the same error yesterday on an old machine. And now I checked,
gobject-introspection isn't installed on it. )

[Csaba Osztrogonác]

(In reply to comment #29)
> (In reply to comment #26)
> > Created attachment 248931 [details]
> > Patch
> > 
> > gst-plugins-openwebrtc source changed back to ToT of EricssonResearch, added
> > --disable-introspection to try to make EFL EWS happy.
> 
> With this patch I got this error:
> 
> cp -f `find .. -name Owr-0.1.gir.h || echo "Owr-0.1.gir.h"` .
>   CC       owr_utils.lo
> cp: missing destination file operand after ‘.’
> 
> ( I got the same error yesterday on an old machine. And now I checked,
> gobject-introspection isn't installed on it. )

So it seems it is impossible to build openwebrtc without gobject-introspection.

[Csaba Osztrogonác]

The EWS has issues during autogen.sh run:

bindings/java/Makefile.am:41: error: HAVE_INTROSPECTION does not appear in AM_CONDITIONAL
bindings/java/Makefile.am: installing './depcomp'
owr/Makefile.am:84: error: HAVE_INTROSPECTION does not appear in AM_CONDITIONAL
autoreconf: automake failed with exit status: 1
autogen.sh failed

But after passing --enable-introspection=no or --disable-introspection
the build step fails with this error:

cp -f `find .. -name Owr-0.1.gir.h || echo "Owr-0.1.gir.h"` .
  CC       owr_utils.lo
cp: missing destination file operand after ‘.’

Here is the Makefile.in:
...
@[e-mail].h: Owr-0.1.gir
@HAVE_INTROSPECTION_TRUE@       xxd -i $< > $@
@HAVE_INTROSPECTION_TRUE@       shasum -a 1 -b < $< | head -c 40 > $<.sha1
@HAVE_INTROSPECTION_TRUE@       xxd -i $<.sha1 >> $@
@HAVE_INTROSPECTION_TRUE@       rm -f $<.sha1
@[e-mail].h:
@HAVE_INTROSPECTION_FALSE@      cp -f `find $(top_srcdir) -name Owr-0.1.gir.h || echo "Owr-0.1.gir.h"` .
...

So we have to fix this buggy openwebrtc or add gobject-introspection.
Philippe, Gyuyoung, please decide which one would be the better.

But you have to know that I won't have any time for WebKit this week since now.
(I can only update the patch if somebody else debug this issue.)

[Csaba Osztrogonác]

Or we should simply disable MEDIA_STREAM on EFL not to block OpenWebRTC
development (bug142476) and we can reenable once we fixed this bug.

[Csaba Osztrogonác]

Created attachment 248939 [details]
Patch

I propose adding gobject-introspection as dependency, because OpenWebRTC isn't buildable at all without it. Gyuyoung, could you install it to the EWS and then push the submit to the EWS button for this patch?

[Philippe Normand]

(In reply to comment #33)
> Created attachment 248939 [details]
> Patch
> 
> I propose adding gobject-introspection as dependency, because OpenWebRTC
> isn't buildable at all without it. Gyuyoung, could you install it to the EWS
> and then push the submit to the EWS button for this patch?

I have a patch fixing the build:
https://github.com/EricssonResearch/openwebrtc/pull/256

I'm fine with waiting the outcome of the patch above.

[Csaba Osztrogonác]

(In reply to comment #34)
> (In reply to comment #33)
> > Created attachment 248939 [details]
> > Patch
> > 
> > I propose adding gobject-introspection as dependency, because OpenWebRTC
> > isn't buildable at all without it. Gyuyoung, could you install it to the EWS
> > and then push the submit to the EWS button for this patch?
> 
> I have a patch fixing the build:
> https://github.com/EricssonResearch/openwebrtc/pull/256
> 
> I'm fine with waiting the outcome of the patch above.

OK, I'll update the hash once it is landed in OpenWebRTC.

[Csaba Osztrogonác]

Created attachment 249323 [details]
Patch

[Csaba Osztrogonác]

(In reply to comment #36)
> Created attachment 249323 [details]
> Patch

updated openwebrtc to https://github.com/WebRTCinWebKit/openwebrtc/commit/1598e6f612d257196db70d0cf930ede94f83497a not to avoid adding gobject-introspection dependency. Let's see what EWS says.

[Philippe Normand]

Comment on attachment 249323 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249323&action=review

> Tools/efl/install-dependencies:88
> +        libv4l-dev \

I think this one is no longer needed either.

> Tools/efl/jhbuild.modules:328
> +  <autotools id="openwebrtc" autogenargs="--enable-bridge=no">

Missing option to disable g-i. IIRC configure errors out if not found but enabled.

[Csaba Osztrogonác]

(In reply to comment #38)
> Comment on attachment 249323 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249323&action=review
> 
> > Tools/efl/install-dependencies:88
> > +        libv4l-dev \
> 
> I think this one is no longer needed either.
Correct, will be removed.

> 
> > Tools/efl/jhbuild.modules:328
> > +  <autotools id="openwebrtc" autogenargs="--enable-bridge=no">
> 
> Missing option to disable g-i. IIRC configure errors out if not found but
> enabled.

It works for me now without this extra option, configure 
script determines properly if it isn't available.

[Csaba Osztrogonác]

Created attachment 249324 [details]
Patch

[Philippe Normand]

Comment on attachment 249324 [details]
Patch

Nice, lgtm

[Gyuyoung Kim]

Comment on attachment 249324 [details]
Patch

Ossy and Philippe, Thank you for supporting OpenWebRTC !

[WebKit Commit Bot]

Comment on attachment 249324 [details]
Patch

Clearing flags on attachment: 249324

Committed r181892: <http://trac.webkit.org/changeset/181892>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.