newrelic / video-agent-iOS

Video Agent for iOS and tvOS
Apache License 2.0
1 stars 9 forks source link

Handle correctly seek events while paused #21

Closed oliviermartin08 closed 11 months ago

oliviermartin08 commented 1 year ago

πŸ“– Description & Context

On iOS, a fix has been recently done to the video tracker in order to improve the seek events. What was done in https://github.com/newrelic/video-agent-iOS/pull/20:

⚠️ However, this fix did not cover correctly when a user made a seek during the pause state.

πŸ‘· Fix

Modified the state isSeekingDuringPlayback for isUserSeeking. With this solution, the lib integrators are now 100% handling when a seek event starts (on a user seek interaction). Then, with the KVO observers, the tracker is handling by itself when the seek ends as well as its buffer. So, right now:

πŸ““ Notes about this solution

oliviermartin08 commented 1 year ago

For the question of Charles on the previous PR here: https://github.com/newrelic/video-agent-iOS/pull/20#discussion_r1226565258

I thought we wouldn't need this condition and would simply rely on isSeekingDuringPlayback changes to call [self sendSeekStart];. Is making isSeekingDuringPlayback KVO compatible a solution that could work?

No, it doesn't really fix it for the case where a user spams the seek button very quickly as mentioned in this PR Notes. ☝️ Basically, I think that we still need an internal state inside the tracker to provide sending multiple seek events when the player is already seeking (goSeekStart function purpose). This basically regroups multiple seek interactions inside a single seek event when the buffering occurs in the same time frame.

miransar commented 11 months ago

@oliviermartin08 PR looks good. As this is a major change due to function names changing, please bump up the version to 2.0.0 here. Thanks.

oliviermartin08 commented 11 months ago

Bumped version to 2.0.0, thanks @miransar πŸ™