Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Live Video Payment Mechanism #3225

Merged
merged 8 commits into from
Nov 5, 2024
Merged

Live Video Payment Mechanism #3225

merged 8 commits into from
Nov 5, 2024

Conversation

leszko
Copy link
Contributor

@leszko leszko commented Oct 28, 2024

It splits the payment mechanism from media transport by creating a new endpoint '/payment' dedicated just for payments.

How will it work with Live Video Transport Protocol?

  1. While sending segment(s), Gateway will call LivePaymentSender.SendPayment()
  2. While receiving segments(s), Orchestrators will call LivePaymentReceiver.AccountSegment()
    • If it returns error, we should stop Realtime Video Stream
  3. We need to pay upfront for a few segments to avoid race condition (if segment data comes before payment)

Notes to reviewers

  • This code is currently not used anywhere, still, I suggest reviewing it and merging to keep PRs small
  • it does not change any existing logic, so for transcoding and AI jobs, the payment is still sent together with the job params

@leszko leszko changed the title [Do not merge] New payment mechanism [Do not merge] Split payment mechanism for Realtime Video Oct 29, 2024
@leszko leszko changed the title [Do not merge] Split payment mechanism for Realtime Video Realtime Video Payment Mechanism Oct 31, 2024
@leszko leszko changed the base branch from ja/ai-live-publish to ai-video October 31, 2024 10:38
It split the payment mechanism from media transport by creating a new endpoint '/payment' dedicated just for payments.
@leszko leszko requested review from j0sh and thomshutt October 31, 2024 10:45
@leszko leszko marked this pull request as ready for review October 31, 2024 10:46
@leszko leszko requested a review from rickstaa as a code owner October 31, 2024 10:46
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 48.16327% with 127 lines in your changes missing coverage. Please review.

Project coverage is 35.77474%. Comparing base (bf10079) to head (d538dd5).
Report is 3 commits behind head on ai-video.

Files with missing lines Patch % Lines
net/lp_rpc.pb.go 28.39506% 58 Missing ⚠️
server/live_payment.go 51.25000% 29 Missing and 10 partials ⚠️
server/segment_rpc.go 66.15385% 22 Missing ⚠️
core/orchestrator.go 0.00000% 5 Missing ⚠️
common/testutil.go 0.00000% 1 Missing ⚠️
server/ai_session.go 0.00000% 1 Missing ⚠️
server/rpc.go 0.00000% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##            ai-video       #3225         +/-   ##
===================================================
+ Coverage   35.73535%   35.77474%   +0.03939%     
===================================================
  Files            124         125          +1     
  Lines          34929       35089        +160     
===================================================
+ Hits           12482       12553         +71     
- Misses         21754       21831         +77     
- Partials         693         705         +12     
Files with missing lines Coverage Δ
net/lp_rpc_grpc.pb.go 9.93789% <ø> (ø)
server/broadcast.go 80.07905% <100.00000%> (+0.01576%) ⬆️
common/testutil.go 16.66667% <0.00000%> (-0.28248%) ⬇️
server/ai_session.go 2.51799% <0.00000%> (+0.04449%) ⬆️
server/rpc.go 67.86787% <0.00000%> (-0.20442%) ⬇️
core/orchestrator.go 72.50308% <0.00000%> (-0.44977%) ⬇️
server/segment_rpc.go 76.95652% <66.15385%> (-2.41095%) ⬇️
server/live_payment.go 51.25000% <51.25000%> (ø)
net/lp_rpc.pb.go 16.22150% <28.39506%> (+0.61109%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 836d006...d538dd5. Read the comment docs.

Files with missing lines Coverage Δ
net/lp_rpc_grpc.pb.go 9.93789% <ø> (ø)
server/broadcast.go 80.07905% <100.00000%> (+0.01576%) ⬆️
common/testutil.go 16.66667% <0.00000%> (-0.28248%) ⬇️
server/ai_session.go 2.51799% <0.00000%> (+0.04449%) ⬆️
server/rpc.go 67.86787% <0.00000%> (-0.20442%) ⬇️
core/orchestrator.go 72.50308% <0.00000%> (-0.44977%) ⬇️
server/segment_rpc.go 76.95652% <66.15385%> (-2.41095%) ⬇️
server/live_payment.go 51.25000% <51.25000%> (ø)
net/lp_rpc.pb.go 16.22150% <28.39506%> (+0.61109%) ⬆️

... and 1 file with indirect coverage changes

server/segment_rpc.go Outdated Show resolved Hide resolved
server/broadcast.go Outdated Show resolved Hide resolved
@leszko leszko requested a review from thomshutt November 4, 2024 08:50
Copy link
Collaborator

@j0sh j0sh left a comment

Choose a reason for hiding this comment

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

Double checking I understand the intent here -

The plan is to replicate the video workflow that sends the payments in as headers?

So in the currently proposed /payments endpoint, the payment data comes in as headers, with an empty body?

One more question - we have been calling this a "live" type of pipeline but here this is called "realtime" - do we want to align those terms?

Left a few small questions / comments through the PR but otherwise LGTM.

server/rpc.go Outdated Show resolved Hide resolved
net/lp_rpc.pb.go Outdated Show resolved Hide resolved
net/lp_rpc_grpc.pb.go Outdated Show resolved Hide resolved
server/segment_rpc.go Show resolved Hide resolved
@leszko
Copy link
Contributor Author

leszko commented Nov 5, 2024

Double checking I understand the intent here -

The plan is to replicate the video workflow that sends the payments in as headers?

So in the currently proposed /payments endpoint, the payment data comes in as headers, with an empty body?

Yes. The payment comes in headers just to reuse what we're currently using for payments. So, you'll send Payment in Headers and an empty body. To integrate it with the Transport protol, we'll need to call:

  • LivePaymentSender.SendPayment() in Gateway anytime a segment (or a couple of segments) is sent
  • LivePaymentReceiver.AccountSegment() in Orchestrator anytime a segment (or a couple of segments) is received

One more question - we have been calling this a "live" type of pipeline but here this is called "realtime" - do we want to align those terms?

Yes, I'll update it to use "Live" instead.

Left a few small questions / comments through the PR but otherwise LGTM.

Thank you 🙏

@leszko leszko changed the title Realtime Video Payment Mechanism Live Video Payment Mechanism Nov 5, 2024
@leszko leszko merged commit 95ecb08 into ai-video Nov 5, 2024
15 checks passed
@leszko leszko deleted the new-payment-mechanism branch November 5, 2024 10:30
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.

3 participants