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

Run MistInHLS mist binary on push end for each rendition #55

Merged
merged 3 commits into from Oct 10, 2022

Conversation

AlexKordic
Copy link
Contributor

@AlexKordic AlexKordic commented Oct 7, 2022

Config config.PathMistDir now contains path to MistServer binaries directory. We use this info to construct absolute path to MistProcLivepeer and MistInHLS.

MistInHLS is used to create .dtsh headers file alongside playlist and segments created on s3.

DDVTECH/mistserver#117 is created for tracking MistServer issue that will obsolete this extra step of running MistInHLS.

Add subprocess/logging.go

Comment on lines -78 to -99
func streamOutput(src io.Reader, dst *bytes.Buffer, out io.Writer) error {
mw := io.MultiWriter(dst, out)
s := bufio.NewReader(src)
for {
var line []byte
line, err := s.ReadSlice('\n')
if err == io.EOF && len(line) == 0 {
break
}
if err == io.EOF {
return fmt.Errorf("Improper termination: %v", line)
}
if err != nil {
return err
}
_, err = mw.Write(line)
if err != nil {
return err
}
}
return nil
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@emranemran moved this to logging.go. Please check for sanity. Removed multiwriter - deemed unnecessary

@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #55 (ce3f3b3) into main (2f85354) will increase coverage by 0.40132%.
The diff coverage is 0.00000%.

❗ Current head ce3f3b3 differs from pull request most recent head e139f46. Consider uploading reports for the commit e139f46 to get more accurate results

Impacted file tree graph

@@                 Coverage Diff                 @@
##                main         #55         +/-   ##
===================================================
+ Coverage   34.88152%   35.28284%   +0.40131%     
===================================================
  Files             17          17                 
  Lines           1055        1043         -12     
===================================================
  Hits             368         368                 
+ Misses           642         630         -12     
  Partials          45          45                 
Impacted Files Coverage Δ
handlers/misttriggers/push_end.go 21.92982% <0.00000%> (-5.24409%) ⬇️
handlers/transcode.go 10.37736% <0.00000%> (+2.52021%) ⬆️

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 2f85354...e139f46. Read the comment docs.

Impacted Files Coverage Δ
handlers/misttriggers/push_end.go 21.92982% <0.00000%> (-5.24409%) ⬇️
handlers/transcode.go 10.37736% <0.00000%> (+2.52021%) ⬆️

@@ -63,7 +66,7 @@ func (d *MistCallbackHandlersCollection) TranscodingPushEnd(w http.ResponseWrite
return
}

uploadSuccess := pushStatus == "null"
uploadSuccess := pushStatus != "null"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing shows we actually get null on error and json object on success.

@@ -76,6 +79,10 @@ func (d *MistCallbackHandlersCollection) TranscodingPushEnd(w http.ResponseWrite
}
}

if err := createDtsh(actualDestination); err != nil {
_ = config.Logger.Log("msg", "createDtsh failed", "err", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we have more context (e.g IDs) on this log line to help identify what it failed for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding actualDestination to that log line

@@ -143,3 +150,24 @@ func uuidFromPushUrl(uri string) (string, error) {
}
return path[len(path)-2], nil
}

func createDtsh(destination string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a unit test for this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea

@AlexKordic AlexKordic merged commit 1cbf6db into main Oct 10, 2022
@hjpotter92 hjpotter92 deleted the ak/create_dtsh_on_end branch May 30, 2023 07:25
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.

None yet

2 participants