Skip to content

Dragging the zoomview during playback is hectic#25

Merged
thom4parisot merged 7 commits intomasterfrom
fix-25
Feb 17, 2014
Merged

Dragging the zoomview during playback is hectic#25
thom4parisot merged 7 commits intomasterfrom
fix-25

Conversation

@thom4parisot
Copy link
Contributor

@thom4parisot thom4parisot commented Feb 17, 2014

Seeking while dragging doesn't seem to work unless you click on it. If you drag the zoomview the overview and zoomview highlighting don't match anymore until you click on it again.

@urinieto
Copy link

urinieto commented Feb 13, 2014

I just notice the same thing. Is there going to be a fix for this soon? Thanks.

@thom4parisot
Copy link
Contributor

thom4parisot commented Feb 14, 2014

Well, dragging the zoomview 1) seeks on click 2) then moves the zoomview without changing the playhead position.

Is it a problem?

Whereas the fact it's not in sync anymore, that is a problem for sure.
Is it something you can reproduce here?

@urinieto
Copy link

urinieto commented Feb 14, 2014

The problem is that the audio is not in sync with the playhead in the zoom view anymore, so that if people want to annotate a segment in detail they can easily get confused by it.

Yes, it is reproducible in that site.

@ghost
Copy link
Author

ghost commented Feb 14, 2014

It probably needs an update of 1 or 2 lines of code on the dragging mouse release event to do what the click does but I am just guessing.

@urinieto
Copy link

urinieto commented Feb 14, 2014

Is this something that will be fixed soon? I would say it's a relatively important bug, since people who would like to annotate an audio signal might get quite confused with it.

If it's not going to be fixed soon, could somebody point me to the code part that should be changed?

Thanks!

@thom4parisot
Copy link
Contributor

thom4parisot commented Feb 14, 2014

@urinieto @martybalandis hey, "soon" is going to be defined: depending how long it takes to fix it I might sort it out tonight or in the train on monday morning. Next week I'm quite busy in team discovery so I can't guaranty finding the time to address this issue.

If you find any way to solve the problem meanwhile, address a pull request. It's not a problem if it's not perfect as long as it solves the problem.

@thom4parisot
Copy link
Contributor

thom4parisot commented Feb 14, 2014

Okay so I had a look. I'm not sure to understand the problem: what you explain is the intended behaviour.

  1. when you click on the zoomview, the playhead moves and the audio element currentTime is updated
  2. when you drag the zoomview, the visible offset is updated in the overview, and the playhead does not move
  3. when you release the zoomview, nothing changes anymore

However when you do that while playing, it just confuses everything. Is that the problem?

If not, don't hesitate to make a screencast with LICEcap or something like that to share it in this issue.

@urinieto
Copy link

urinieto commented Feb 14, 2014

Yes, the problem is that the audio that you hear and the playhead are not synchronized. You might be listening to silence but you can see how the playhead in he zoom view is going through some waves that should be audible.

@urinieto
Copy link

urinieto commented Feb 14, 2014

When you press stop, and play again, the problem is solved.

@thom4parisot
Copy link
Contributor

thom4parisot commented Feb 14, 2014

So we have two things:

  1. do you want the playhead to be moved when the drag start while there is no playback?
  2. do you want the playhead to be moved when the drag start during playback?

I guess there are some race conditions because offset and playhead values are updated from two different ways (by the user and the audio element) which causes this behaviour.

My bet is to make the playhead move only on mouse up, if it's not happening after a drag.

@urinieto
Copy link

urinieto commented Feb 14, 2014

My bet is to make the playhead move only on mouse up, if it's not happening after a drag.

That would be ideal. But I wouldn't care about 1 or 2 as long as the playhead is always located in the right place.

@thom4parisot
Copy link
Contributor

thom4parisot commented Feb 15, 2014

Okay I checked a little bit this morning and it's not that trivial. When the media element is not playing, that's easy to do.

It's rather when it's playing that things starts to become harder. The playhead needs to keep in sync with the offset in the animation loop. It requires a bit of work and I'll check that monday morning.

@urinieto
Copy link

urinieto commented Feb 15, 2014

Thanks a lot man, I really appreciate your help.

On Sat, Feb 15, 2014 at 9:09 AM, Thomas Parisot notifications@github.comwrote:

Okay I checked a little bit this morning and it's not that trivial. When
the media element is not playing, that's easy to do.

It's rather when it's playing that things starts to become harder. The
playhead needs to keep in sync with the offset in the animation loop. It
requires a bit of work and I'll check that monday morning.

Reply to this email directly or view it on GitHubhttps://github.com//issues/25#issuecomment-35156971
.

@thom4parisot
Playhead position change only on click release.
84aa071 
And only if the user was not dragging.
@thom4parisot
Removed unecessary drawing call during sync syncPlayhead.
010a555
@thom4parisot
Initialised playheadPixel value.
c25c1a3 
- fixed playhead pixel display after dragging with no playback and no seeking
@thom4parisot
Tweaked dev canvas height.
5e0aa08 
And thinking to have that "per view".
@thom4parisot
Playhead animation is no more bound to the frame seek.
efae671 
Basically:
- playhead position is calculated anyway, displaying it or hiding it if necessary
- `newFrame` calculation is going to be performed on playback heartbeat rather than on playhead progress
@thom4parisot
Frame seeking on playback.
a22a64c
@thom4parisot
Copy link
Contributor

thom4parisot commented Feb 17, 2014

Hey guys,

so I made a couple of tweaks:

  • playhead position always take in account the actual offset so as when dragging, it is displayed at the right time even during playback
  • the frame move is now triggered by the playback heartbeat and not by the playhead move
  • on click, the playhead move is performed on mouseup, if there is no dragging involved

It's time to checkout the branch and provide your feedbacks now :-)

@urinieto
Copy link

urinieto commented Feb 17, 2014

This is great, this totally fixes the issue. Thanks a lot!

I have another related request (I wouldn't call it a bug though):

  • When clicking on the zoom view in order to change the location of the playhead, the entire view is "moved", apparently in a random way. This only happens when the zoom view is showing the middle of the signal (i.e. it's not in the beginning nor the end). From a user point of view, it is annoying to see the view change completely when you only want to place the playhead on top of a specific location.

Do you think this behavior would be better? If so, would it be a hard modification to make?

Thanks a lot for your help!

@thom4parisot
Copy link
Contributor

thom4parisot commented Feb 17, 2014

Yes I was considering that change. It makes sense from the overview but not when you remain in the same area of the zoom view. I'll open another issue to state that intent.

thom4parisot pushed a commit that referenced this pull request Feb 17, 2014
Merge pull request #25 from bbcrd/fix-25
6e0dc63 
Dragging zoomview bug (low priority)
@thom4parisot thom4parisot merged commit 6e0dc63 into master Feb 17, 2014
@thom4parisot thom4parisot deleted the fix-25 branch February 17, 2014 19:20
@thom4parisot thom4parisot added this to the 0.0.5 milestone Feb 17, 2014
@urinieto
Copy link

urinieto commented Feb 17, 2014

Awesome. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

0.0.5

Development

Successfully merging this pull request may close these issues.

2 participants

@thom4parisot @urinieto