Page MenuHomePhabricator

Efl.Player: Add setter of playback_progress
ClosedPublic

Authored by jsuya on Dec 19 2019, 9:27 PM.

Details

Summary

The setter of Efl.Player.playback_progress is implemented in each class below.
Efl.Ui.Animation_View
Efl.Ui.Image
Efl.Ui.Image_Zoomable
Efl.Canvas.Video

ref T8476
Depends on D10915

Test Plan

N/A

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jsuya created this revision.Dec 19 2019, 9:27 PM
jsuya requested review of this revision.Dec 19 2019, 9:27 PM
jsuya added inline comments.Dec 20 2019, 1:23 AM
src/lib/efl/interfaces/efl_player.eo
73

@segfaultxavi
How do I write a version? ( @since 1.24 )
As far as I know that set and get docs are not displayed.

I am OK with this API (it also compiles and passes tests).

I'll let somebody else review the code.

src/lib/efl/interfaces/efl_player.eo
73

You're right. The getter and setter docs are currently added to the property docs (at least for mono_gen) but not the @since information.

Add the tag here (in the setter) and we'll create another task to handle this.

bu5hm4n added inline comments.Dec 20 2019, 8:03 AM
src/lib/elementary/efl_ui_image.c
1877

Why dividing by frame_duration?

src/lib/elementary/efl_ui_image_zoomable.c
3196

Same question as above.

jsuya updated this revision to Diff 27747.Dec 22 2019, 10:37 PM

@bu5hm4n You are right. This code is weird.
The frame_duration was already used in progress_get and I referenced it.

_efl_ui_image_efl_player_playback_progress_get(const Eo *obj EINA_UNUSED, Efl_Ui_Image_Data *sd)
...
     return (sd->cur_frame * sd->frame_duration) / sd->frame_count;

I modified it together.

But in efl_ui_image, frame_duration is interval with one frame.
In vg, frame_duration is the interval from the start frame to the end frame.
In particular, there seems to be a problem with position_set/get in efl_ui_image.
I hope someone will check.

jsuya marked 4 inline comments as done.Dec 22 2019, 10:38 PM
This comment was removed by jsuya.
jsuya updated this revision to Diff 27758.Dec 23 2019, 1:06 AM
jsuya edited the summary of this revision. (Show Details)

Fix dependent patch number

zmike requested changes to this revision.Dec 23 2019, 7:48 AM
zmike added a subscriber: zmike.
zmike added inline comments.
src/lib/elementary/efl_ui_image.c
1863

This is a good catch, but shouldn't it be (sd->frame_count - 1) and then fixed to not divide by 0?

1874

Shouldn't this be progress * (sd->frame_count - 1) since it's zero-indexed?

src/lib/elementary/efl_ui_image_zoomable.c
3195

Same as above.

This revision now requires changes to proceed.Dec 23 2019, 7:48 AM
bu5hm4n resigned from this revision.Dec 23 2019, 8:20 AM

My concerns have been met.

jsuya updated this revision to Diff 27787.Dec 23 2019, 5:43 PM

@zmike thank you for review
I have updated the code.

jsuya marked 3 inline comments as done.Dec 23 2019, 5:43 PM
zmike accepted this revision.Dec 30 2019, 8:20 AM

Seems good to go.

This revision is now accepted and ready to land.Dec 30 2019, 8:20 AM
This revision was automatically updated to reflect the committed changes.