Page MenuHomePhabricator

efl: remove implementation of Efl.Player
AbandonedPublic

Authored by Jaehyun_Cho on Sep 19 2019, 3:39 AM.

Details

Summary

Interface Efl.Player has media player's all possible methods.
However, efl's image, video, and animation classes implement Efl.Player.
As a result, efl's image, video, and animation classes implement just
some of the methods of Efl.Player.

Since basically a class that implements an interface must implement all
the methods of the interface, it seems that either interface Efl.Player
has too many methods or implementing interface Efl.Player is not
appropriate.

This patch removes the implementation of the interface Efl.Player.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13397
Build 9441: arc lint + arc unit
Jaehyun_Cho created this revision.Sep 19 2019, 3:39 AM
Jaehyun_Cho requested review of this revision.Sep 19 2019, 3:39 AM

Is it ok if this is submitted?

This creates in one go a lot of new API that we would have to test, review etc. is it possible to wait with this until after the release ? The Efl.Player interface is not scheduled to be stable in this release cycle anyways.

@bu5hm4n Thank you for your reply.

To use animation, it is necessary to confirm (modify or remove) Efl.Player and its related APIs.
Since there are big demands on use of animation in Tizen, can we discuss and confirm Efl.Player and its related APIs after the release with higher priority?

zmike added a comment.Mon, Sep 23, 7:06 PM

Hm, I'm unsure how we could be doing this release without Efl.Player, now that I look at it. Efl.Ui.Image and Image_Zoomable both use it, so this is a hard dependency. Also Efl.Gfx.View, which we seem to have missed, though that's something else.

I don't like the idea of reimplementing existing interface methods on a class like this. I think since this only affects one class directly (Efl.Ui.Image) and the interface itself is already pretty reasonable we can just add it to the list.

zmike requested changes to this revision.Mon, Sep 23, 7:06 PM
This revision now requires changes to proceed.Mon, Sep 23, 7:06 PM

@zmike
I introduced this patch because many methods of Efl.Player are not fully implemented in its inheriting classes. It is related to T5719.
Is it ok if classes do not fully implement their interface's methods?

zmike added a comment.Mon, Sep 23, 7:28 PM

It is not ok, but if you see my comment in the Efl.Player ticket I think there would not be much work needed to fill those in since this is not used in many stable classes. This is assuming we split the interface.

bu5hm4n added a comment.EditedMon, Sep 23, 11:08 PM

What? Why are we hearing for the first time that these classes should be made stable ? There are quite a few things around there that need discussion and work. Docs are lagging, a property like auto_del should probebly not exist, The whole animation classes are not using the coordinate systems we are normally using in unified API (Like Eina.Position2D or Eina.Vector)

EDIT: i was probebly not fully awake when i read you comment Jaehyun_Cho, I missed that you want to do that *after* the release, which makes sense IMO. I will create a task with things that i think are not perfect on the animation framework, so we can work on that, ok ? :)

yes, thank you :)

Jaehyun_Cho abandoned this revision.Sun, Sep 29, 7:38 PM

This patch is not appropriate and the alternative patch exists.