Bug 175576

Summary: [GStreamer] The glvideoflip GStreamer element isn't available. Video mirroring and rotation functionalities are thus disabled.
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, clopez, cturner, magomez, mcatanzaro
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch clopez: review+

Description Michael Catanzaro 2017-08-15 09:15:49 PDT
I see this error in stderr for a bunch of tests on the GTK release bot:

The glvideoflip GStreamer element isn't available. Video mirroring and rotation functionalities are thus disabled.

That's a WebKit bug since the glvideoflip GStreamer element needs to be either built in our JHBuild environment or installed by install-dependencies. What's supposed to be pulling it in?
Comment 1 Carlos Alberto Lopez Perez 2017-08-15 09:22:58 PDT
(In reply to Michael Catanzaro from comment #0)
> I see this error in stderr for a bunch of tests on the GTK release bot:
> 
> The glvideoflip GStreamer element isn't available. Video mirroring and
> rotation functionalities are thus disabled.
> 
> That's a WebKit bug since the glvideoflip GStreamer element needs to be
> either built in our JHBuild environment or installed by
> install-dependencies. What's supposed to be pulling it in?

You need to have graphene installed when building gstreamer*, so the glvideoflip automatically gets built.

graphene is still not available on debian stretch, its on testing.

I have been told this warning is harmless and you are safe to ignore it (unless you care about video mirroring/rotation functionalities of course)
Comment 2 Michael Catanzaro 2017-08-15 09:33:41 PDT
It's cluttering our layout test results, which is not acceptable.

I think we'll need to add graphene to our JHBuild environment, and have the right GStreamer plugins package (-base? -good? -bad?) depend on it.
Comment 3 Carlos Alberto Lopez Perez 2017-08-15 09:50:58 PDT
(In reply to Michael Catanzaro from comment #2)
> It's cluttering our layout test results, which is not acceptable.
> 
> I think we'll need to add graphene to our JHBuild environment, and have the
> right GStreamer plugins package (-base? -good? -bad?) depend on it.

bad.
Comment 4 Xabier Rodríguez Calvar 2017-08-16 00:58:47 PDT
FYI, this warning is not WebKit output but IIRC OpenWebRTC one.
Comment 5 Michael Catanzaro 2017-08-28 10:27:22 PDT
Created attachment 319180 [details]
Patch
Comment 6 Michael Catanzaro 2017-08-28 10:27:50 PDT
There's still a huge number of tests with stderr output, but this cuts fixes ~15 or so.
Comment 7 Carlos Alberto Lopez Perez 2017-08-28 11:17:50 PDT
Comment on attachment 319180 [details]
Patch

I think that instead of doing this, we can add a patch for openwebrtc on the jhbuild recipe that comments out the warning (its at local/owr_video_renderer.c). That way we keep the jhbuild smaller and quicker to build.
Comment 8 Michael Catanzaro 2017-08-28 11:35:38 PDT
graphene and meson take almost no time to build since they're small and don't use Autotools!

A patch would work too, but I have a suspicion that openwebrtc would not be complaining about the missing element if it were not important for WebRTC.
Comment 9 Carlos Alberto Lopez Perez 2017-08-28 12:24:43 PDT
(In reply to Michael Catanzaro from comment #8)
> graphene and meson take almost no time to build since they're small and
> don't use Autotools!
> 

Ok.

Can you at least add a  <pkg-config> entry to avoid building graphene when the developer already installed it from the distro?

> A patch would work too, but I have a suspicion that openwebrtc would not be
> complaining about the missing element if it were not important for WebRTC.

Then I hope to see some new test passing after this lands like webrtc/video-rotation.html :)
Comment 10 Michael Catanzaro 2017-08-28 14:08:05 PDT
(In reply to Carlos Alberto Lopez Perez from comment #9)
> Ok.
> 
> Can you at least add a  <pkg-config> entry to avoid building graphene when
> the developer already installed it from the distro?

No, because graphene depends on glib, and glib is in our jhbuild environment. The system version of graphene could easily require newer symbols than are provided by our glib.
Comment 11 Carlos Alberto Lopez Perez 2017-08-28 14:22:01 PDT
(In reply to Michael Catanzaro from comment #10)
> (In reply to Carlos Alberto Lopez Perez from comment #9)
> > Ok.
> > 
> > Can you at least add a  <pkg-config> entry to avoid building graphene when
> > the developer already installed it from the distro?
> 
> No, because graphene depends on glib, and glib is in our jhbuild
> environment. The system version of graphene could easily require newer
> symbols than are provided by our glib.

Mmm..... good point.
Comment 12 Carlos Alberto Lopez Perez 2017-08-28 14:22:26 PDT
Comment on attachment 319180 [details]
Patch

Please watch the bots after landing for possible breakages.
Comment 13 Michael Catanzaro 2017-08-28 17:28:55 PDT
OK, I'll watch.
Comment 14 Michael Catanzaro 2017-08-28 17:40:39 PDT
Committed r221284: <http://trac.webkit.org/changeset/221284>
Comment 15 Michael Catanzaro 2017-08-28 18:42:57 PDT
Unfortunately release bot has died for unrelated reasons, and all other GTK test bots are behind. The GTK buildbots are fine and the WPE bots are fine. I think it was a low-risk change, though.
Comment 16 Xabier Rodríguez Calvar 2017-08-29 03:33:08 PDT
(In reply to Michael Catanzaro from comment #8)
> A patch would work too, but I have a suspicion that openwebrtc would not be
> complaining about the missing element if it were not important for WebRTC.

If it's the code I wrote and I think it is, the only thing is that some screen video flip won't work. Summing up, its importance is very relative.