Skip to content

Cleanup for the media tracking PR#105

Merged
yangyansong-adbe merged 5 commits intoadobe:mediafrom
yangyansong-adbe:fix_todos
Nov 8, 2023
Merged

Cleanup for the media tracking PR#105
yangyansong-adbe merged 5 commits intoadobe:mediafrom
yangyansong-adbe:fix_todos

Conversation

@yangyansong-adbe
Copy link
Copy Markdown
Contributor

@yangyansong-adbe yangyansong-adbe commented Nov 7, 2023

  • update the dev app for testing media APIs
  • small cleanups for the previous media tracking PR

@yangyansong-adbe yangyansong-adbe changed the title Fix todos Cleanup for the media tracking PR Nov 7, 2023
end if

sessionId = m._private.mediaSession.startNewSession()
playhead = 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a position variable on L285, can we check if the playhead is valid there and combine this logic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The L285 code is used to handle the position/playhead of the previous session. It might be confusing to mix them together.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will get getting Playhead from customer right? So ideally we dont need any playhead logic. This is for fallback? What about sessionEnd?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline and decided to move this logic from the API layer to the SDK (task node) layer in another PR.

@yangyansong-adbe yangyansong-adbe merged commit 78e892e into adobe:media Nov 8, 2023
@yangyansong-adbe yangyansong-adbe deleted the fix_todos branch January 16, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants