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

Upload transcodes to output target location rather than input location #61

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

thomshutt
Copy link
Contributor

@thomshutt thomshutt commented Oct 13, 2022

Puts all transcoded segments + manifests under a transcoded/ dir, rather than alongside the source MP4.

Doesn't account for multiple output locations yet, but is enough for us to test with.

image

@codecov
Copy link

codecov bot commented Oct 13, 2022

Codecov Report

Merging #61 (a83a11e) into main (6666234) will increase coverage by 0.05715%.
The diff coverage is 40.00000%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main         #61         +/-   ##
===================================================
+ Coverage   35.42958%   35.48673%   +0.05715%     
===================================================
  Files             17          17                 
  Lines           1129        1130          +1     
===================================================
+ Hits             400         401          +1     
  Misses           682         682                 
  Partials          47          47                 
Impacted Files Coverage Δ
handlers/misttriggers/live_tracklist.go 0.00000% <0.00000%> (ø)
handlers/misttriggers/push_end.go 29.23077% <0.00000%> (+0.22314%) ⬆️
handlers/transcode.go 6.04396% <0.00000%> (ø)
cache/segmenting.go 74.19355% <100.00000%> (ø)
handlers/misttriggers/triggers.go 73.52941% <100.00000%> (+0.80213%) ⬆️
handlers/upload.go 49.33333% <100.00000%> (ø)

Continue to review full report at Codecov.

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

Impacted Files Coverage Δ
handlers/misttriggers/live_tracklist.go 0.00000% <0.00000%> (ø)
handlers/misttriggers/push_end.go 29.23077% <0.00000%> (+0.22314%) ⬆️
handlers/transcode.go 6.04396% <0.00000%> (ø)
cache/segmenting.go 74.19355% <100.00000%> (ø)
handlers/misttriggers/triggers.go 73.52941% <100.00000%> (+0.80213%) ⬆️
handlers/upload.go 49.33333% <100.00000%> (ø)

@thomshutt thomshutt marked this pull request as ready for review October 13, 2022 22:10
CallbackUrl string
UploadDir string
CallbackURL string
UploadURL string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed Url -> URL because that's Go convention.

Changed "Dir" to URL because it's a URL and not a Dir

@@ -36,6 +36,7 @@ func (d *MistCallbackHandlersCollection) Trigger() httprouter.Handle {
_ = config.Logger.Log(
"msg", "Received Mist Trigger",
"trigger_name", triggerName,
"payload", string(payload),
Copy link
Collaborator

Choose a reason for hiding this comment

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

good idea

@emranemran emranemran merged commit a19b232 into main Oct 14, 2022
@@ -107,11 +100,11 @@ func RunTranscodeProcess(mistClient clients.MistAPIClient, request TranscodeSegm
return fmt.Errorf("failed to start MistProcLivepeer: %s", err)
}

dir, _ := url.Parse(".")
uploadDir := inputUrl.ResolveReference(dir)
dir, _ := url.Parse("transcoded/")
Copy link
Member

Choose a reason for hiding this comment

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

does it need to be hard-coded or can it be configurable in request payload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least initially, hardcoded - can look at making it variable in the future if needed

@thomshutt thomshutt deleted the upload-to-correct-dir branch October 14, 2022 10:32
iameli added a commit that referenced this pull request Feb 7, 2023
* Add playback redirection to the closest Mist node

* Update for `http-addr` flag and Mist Load Balancer 'video+' prefix

* Add E2E test for stream redirection

* Remove unused params in E2E Test

* Update cmd/catalyst-node/catalyst-node.go

Co-authored-by: Eli Mallon <[email protected]>

* Update cmd/catalyst-node/catalyst-node.go

Co-authored-by: Eli Mallon <[email protected]>

* Update cmd/catalyst-node/catalyst-node_test.go

Co-authored-by: Eli Mallon <[email protected]>

* Update cmd/catalyst-node/catalyst-node.go

Co-authored-by: Eli Mallon <[email protected]>

* Minor fixes after code review

* Handle Mist returning "FULL" from LoadBalancer API

* go fmt

* Fix Revive warnings

* Fix CI builds

Co-authored-by: Eli Mallon <[email protected]>
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