Bug 44098 – [EFL] Support to enable HTML5's Video based on gstreamer in WebKit-EFL

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

[Gyuyoung Kim]

Let's support to enable HTML5's video in WebKit-EFL.

[Gyuyoung Kim]

Created attachment 64571 [details]
Patch

I try to support HTML5's video in WebKit-EFL. I investigated if EFL supports video / audio. But, I am not sure if EFL supports multimedia framework. Is the emotion library still alive ? If EFL can support video framework, IMO, we need to support HTML5's video using the framework.

In this bug, I make a patch to support HTML5's video using gstreamer. I think WebKit-EFL can use gstreamer. If there are multimedia framework in EFL or WebKit EFL can't use gstreamer, please let me know.

Thanks,
Gyuyoung Kim

[Kenneth Rohde Christiansen]

Cmake guys, how does this look?

[Antonio Gomes]

I'd are rather defer approval for the other CMake heroes. ProFusion?

ps: Kim, please you can add more than one person to the CC list all at once, separated bug "," :-)

[Antonio Gomes]

(In reply to comment #3)
> I'd are rather defer approval for the other CMake heroes. ProFusion?

*I'd rather*

[Rafael Antognolli]

Gyuyoung,

I don't know if the current state of emotion is good enough to try to enable video support using it. So I would also stay with gstreamer. But for using gstreamer, you should also set the ENABLE_GLIB_SUPPORT.

You can do something similar to what is done when enabling the soup backend. Just check the cmake/OptionsEfl.cmake, in the "IF (NETWORK_BACKEND STREQUAL "soup") section.

Other than that, your patch just didn't compile here on my machine. Does it work fine for you, really enabling video support?

Leandro, do you have any other comments regarding the additions to the cmake building system?

[Leandro Pereira]

(In reply to comment #5)
> 
> Leandro, do you have any other comments regarding the additions to the cmake 
> building system?
>

Nope, patch seems fine. Gyuyoung, just send another that forces ENABLE_GLIB_SUPPORT like Rafael said and you'll get my informal r+.

[Gyuyoung Kim]

Created attachment 65257 [details]
Patch

Hello Rafael,

I modify this patch according to your point. I add ENABLE_GLIB_SUPPORT condition for ENABLE_VIDEO as below,

383 +IF (ENABLE_VIDEO)
384 +  SET(ENABLE_GLIB_SUPPORT 1)
385 +  MESSAGE("Forcing Glib support")
386 +ENDIF()  
387 +

I tested this patch in http://www.html5demos.com/video. Video is played on EWebLauncher.
However, video should be tested further. So, I cannot enable video as default yet.

Unfortunately, recently, gtk guy added a patch for fullscreen video. The patch influences on this patch. So,I need to solve the problem first. I make a bug for the issue. (https://bugs.webkit.org/show_bug.cgi?id=44508). I think this bug can be reviewed after finishing the new bug.

Thanks,
Gyuyoung Kim

[Rafael Antognolli]

Ok, the patch seems nice now (besides two trailing whitespaces after this "endif":

+IF (ENABLE_VIDEO)
+  SET(ENABLE_GLIB_SUPPORT 1)
+  MESSAGE("Forcing Glib support")
+ENDIF()  

I'm waiting for bug 44508 get fixed to then try your patch again.

[Gyuyoung Kim]

Created attachment 65394 [details]
Remove whitespace

Ok, I remove the whitespace.

[Gyuyoung Kim]

Bug 44508's patch was landed. Please review this patch. :)

[Rafael Antognolli]

(In reply to comment #10)
> Bug 44508's patch was landed. Please review this patch. :)

Ok, as I told before, the patch is nice, and I just tried it. And it works!

Did you have any problems with the controls (progress bar and play/pause buttons)?

Anyway, if this still needs any fix, I think it can come in a later patch. The current patch is good for landing IMHO.

[Gyuyoung Kim]

(In reply to comment #11)
> (In reply to comment #10)
> > Bug 44508's patch was landed. Please review this patch. :)
> 
> Ok, as I told before, the patch is nice, and I just tried it. And it works!
> 
> Did you have any problems with the controls (progress bar and play/pause buttons)?
> 
> Anyway, if this still needs any fix, I think it can come in a later patch. The current patch is good for landing IMHO.

I think so. Play / Pause button is working well in the "http://www.html5demos.com/video site. However, I need to check if html5 video should be tested further.
I'd like to enable "ENABLE_VIDEO" as default after verifing this further.

For the time being, I also want to land this patch fot testing html5 video. :)

[Gyuyoung Kim]

Hello kenneth and Tonikitoo,

Could you please review this patch ?

[Kenneth Rohde Christiansen]

Comment on attachment 65394 [details]
Remove whitespace

Seems fine with me, but shouldn't it be called GStreamer? and not Gstreamer?

[Gyuyoung Kim]

(In reply to comment #14)
> (From update of attachment 65394 [details])
> Seems fine with me, but shouldn't it be called GStreamer? and not Gstreamer?

It seems "GStreamer" is right. Should I change "Gstreamer" with "GStreamer" in this patch ?

[Kenneth Rohde Christiansen]

yes, please

[Gyuyoung Kim]

Created attachment 66871 [details]
Patch

I change "Gstreamer" with "GStreamer". 

Thank you.

[WebKit Commit Bot]

Comment on attachment 66871 [details]
Patch

Clearing flags on attachment: 66871

Committed r66980: <http://trac.webkit.org/changeset/66980>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.