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.

Sign up for GitHub

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

Jump to bottom

hybrik: fix bug with improper connections #211

Merged
merged 2 commits into from
Jun 17, 2019
Merged

hybrik: fix bug with improper connections #211

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70058fe
hybrik: fix bug with improper connections
zsiec Jun 15, 2019
16eae21
hybrik: fix misspelling
zsiec Jun 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions provider/hybrik/hybrik.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ import (

const (
// Name describes the name of the transcoder
Name = "hybrik"
queued = "queued"
active = "active"
completed = "completed"
failed = "failed"
activeRunning = "running"
activeWaiting = "waiting"
hls = "hls"
Name = "hybrik"
queued = "queued"
active = "active"
completed = "completed"
failed = "failed"
activeRunning = "running"
activeWaiting = "waiting"
hls = "hls"
transcodeElementIDTemplate = "transcode_task_%s"
)

var (
Expand Down Expand Up @@ -110,7 +111,7 @@ func (hp *hybrikProvider) mountTranscodeElement(elementID, id, outputFilename, d

// create the transcode element
e = hwrapper.Element{
UID: "transcode_task" + elementID,
UID: fmt.Sprintf(transcodeElementIDTemplate, elementID),
Kind: "transcode",
Task: &hwrapper.ElementTaskOptions{
Name: "Transcode - " + preset.Name,
Expand Down Expand Up @@ -158,6 +159,7 @@ func makeGetPresetRequest(hp *hybrikProvider, presetID string, ch chan *presetRe

func (hp *hybrikProvider) presetsToTranscodeJob(job *db.Job) (string, error) {
elements := []hwrapper.Element{}
transcodeElementIds := []string{}
var hlsElementIds []int

// create a source element
Expand Down Expand Up @@ -224,18 +226,21 @@ func (hp *hybrikProvider) presetsToTranscodeJob(job *db.Job) (string, error) {
var segmentDur uint
// track the hls outputs so we can later connect them to a manifest creator task
if len(preset.Payload.Targets) > 0 && preset.Payload.Targets[0].Container.Kind == hls {
hlsElementIds = append(hlsElementIds, len(elements))
hlsElementIds = append(hlsElementIds, len(transcodeElementIds))
segmentDur = job.StreamingParams.SegmentDuration
}

e := hp.mountTranscodeElement(strconv.Itoa(len(elements)), job.ID, output.FileName, hp.config.Destination, segmentDur, preset)
taskIndex := strconv.Itoa(len(transcodeElementIds))
e := hp.mountTranscodeElement(taskIndex, job.ID, output.FileName, hp.config.Destination, segmentDur, preset)

transcodeElementIds = append(transcodeElementIds, e.UID)
elements = append(elements, e)
}

// connect the source element to each of the transcode elements
transcodeSuccessConnections := make([]hwrapper.ToSuccess, len(elements))
for i := range elements {
transcodeSuccessConnections[i] = hwrapper.ToSuccess{Element: "transcode_task" + strconv.Itoa(i)}
transcodeSuccessConnections := make([]hwrapper.ToSuccess, len(transcodeElementIds))
for i, id := range transcodeElementIds {
transcodeSuccessConnections[i] = hwrapper.ToSuccess{Element: id}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we range through elements and add element.UID to the transcodeSuccessConnections slice? Something like:

for i, e := range elements {
  transcodeSuccessConnections[i] = hwrapper.ToSuccess{Element: e.UID}
}

Then we don't need an additional slice, I think. And that would also keep the logic to generate the element id inside the monutTranscodeElement function.

If we do that, can you also make the constant transcodeElementIDTemplate local to that function?

Copy link
Member Author

@zsiec zsiec Jun 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This change was made because elements holds a slice of all elements and in this case, not only the transcode task elements, but at least one source element created and appended before we run the logic to create the transcode task elements. (this was causing incorrect connections to be generated)

Rather than assume we have a single source element and doing a range over elements[1:] (along with assuming there are no elements between the source and the transcode tasks), I thought it would be less error prone to explicitly track transcode tasks. This allows us to freely add elements before the transcode elements without having to change that slice offset.

We can safely change
transcodeElementIds = append(transcodeElementIds, fmt.Sprintf(transcodeElementIDTemplate, taskIndex))
to
transcodeElementIds = append(transcodeElementIds, e.UID)

but we make use of transcodeElementIDTemplate again in presetsToTranscodeJob when generating connections to packagings tasks (line 293) (perhaps we could send the transcodeElementIDTemplate into mountTranscodeElement to keep it out of the global scope?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for clarifying! This makes sense, my bad x)

We can keep the code as is :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Cheers! I was tripped up by the code a few times myself, I think we can clean it up a little :)

}

// create the full job structure
Expand Down Expand Up @@ -285,7 +290,7 @@ func (hp *hybrikProvider) presetsToTranscodeJob(job *db.Job) (string, error) {

var manifestFromConnections []hwrapper.ConnectionFrom
for _, hlsElementID := range hlsElementIds {
manifestFromConnections = append(manifestFromConnections, hwrapper.ConnectionFrom{Element: "transcode_task" + strconv.Itoa(hlsElementID)})
manifestFromConnections = append(manifestFromConnections, hwrapper.ConnectionFrom{Element: fmt.Sprintf(transcodeElementIDTemplate, strconv.Itoa(hlsElementID))})
}

cj.Payload.Connections = append(cj.Payload.Connections,
Expand Down