viewer: Hide overlays when playing
ClosedPublic

Authored by stefanzzz on Jul 27 2017, 7:33 AM.

Details

Summary

Make sure only the sink widget remains visible when playing and all
the overlays are hidden. When using keyframes with the transformation
properties, the properties of the overlay are updated with a delay,
which causes the sink widget and the overlay to get out of sync. Hiding
the overlay will hide this problem.

Depends on D1810

Diff Detail

Repository
rPTV Pitivi
stefanzzz created this revision.Jul 27 2017, 7:33 AM
aleb added inline comments.Jul 27 2017, 8:09 AM
pitivi/viewer/viewer.py
471

Why only when the old state was PLAYING?

stefanzzz added inline comments.Jul 27 2017, 8:24 AM
pitivi/viewer/viewer.py
471

Because they will be hidden only if the old sate is PLAYING. It would be the same thing if I always do it though.

aleb added inline comments.Jul 27 2017, 8:33 AM
pitivi/viewer/viewer.py
471

I'm not sure if this is a problem, but for example when a pipeline operation fails, and the pipeline is reset (to NULL and then to PAUSED) - do we need to show_overlays() here?

stefanzzz updated this revision to Diff 6156.Jul 27 2017, 8:35 AM
stefanzzz edited the summary of this revision. (Show Details)

Minor changes.

stefanzzz added inline comments.Jul 27 2017, 8:40 AM
pitivi/viewer/viewer.py
471

Afaik, the pipeline should go through all the intermediary states, so from PLAYING it should always go to PAUSED. But I changed the code, as it's safer to not make this assumption.

aleb added inline comments.Jul 27 2017, 9:01 AM
pitivi/viewer/viewer.py
471

Ah, yes, you are right, if it always goes through all the intermediary steps, we only care about about PAUSED -> PLAYING and PLAYING -> PAUSED.
It should be fine like this, since show_overlays checks self.__hide_all_overlays.

But why add this mechanism in OverlayStack instead of in viewer, where we can hide self.overlay_stack?

stefanzzz added inline comments.Jul 27 2017, 9:13 AM
pitivi/viewer/viewer.py
471

The overlay_stack also contains the sink widget. Hiding it will hide the video preview as well.

Lgtm.

So you are still connected to the deep-notify signals for the spinbuttons?

pitivi/viewer/viewer.py
471

Element do always go through intermediary states.

Lgtm.

So you are still connected to the deep-notify signals for the spinbuttons?

Yes. Otherwise, the spinbuttons won't update if the source properties are changed from another place (like when dragging the viewer). Basically, the values of the spin buttons and viewer will be updated in 3 cases:

  1. Selection is changed
  2. A source property is changed
  3. The position is changed and we use keyframes.
aleb accepted this revision.Aug 1 2017, 11:45 PM
This revision is now accepted and ready to land.Aug 1 2017, 11:45 PM
aleb requested changes to this revision.Aug 13 2017, 6:21 PM
aleb added inline comments.
pitivi/viewer/overlay_stack.py
126–131

If you make this check here, the loop below is skipped and overlay.hide() will not be executed when it should be

This revision now requires changes to proceed.Aug 13 2017, 6:21 PM
stefanzzz added inline comments.Aug 14 2017, 2:28 PM
pitivi/viewer/overlay_stack.py
126–131

All the overlays will be hidden if self.__hide_all_overlays is True. I make sure of that in lines 143-144. So, there's no need to execute that loop.

aleb accepted this revision.Aug 14 2017, 4:28 PM
aleb added inline comments.
pitivi/viewer/overlay_stack.py
126–131

Ok!

This revision is now accepted and ready to land.Aug 14 2017, 4:28 PM
This revision was automatically updated to reflect the committed changes.