concatenate segments #491
Merged
concatenate segments #491
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emranemran
force-pushed
the
emran/concat-ts-segments
branch
2 times, most recently
from
d84c111
to
3761489
Compare
emranemran
force-pushed
the
emran/concat-ts-segments
branch
from
3761489
to
97a50d7
Compare
mjh1
reviewed
Feb 28, 2023
| func (s *TSegmentList) AddSegmentData(segIdx int, data []byte) { | ||
| s.mu.Lock() | ||
| s.SegmentDataTable[segIdx] = data | ||
| s.mu.Unlock() |
There was a problem hiding this comment.
you could do this with a defer like your other examples
emranemran
force-pushed
the
emran/concat-ts-segments
branch
from
97a50d7
to
ad403c9
Compare
|
Updates:
Also, looks like lint caught some issues that I'll need to address as well. |
Codecov Report
@@ Coverage Diff @@
## main #491 +/- ##
===================================================
- Coverage 55.04615% 54.41578% -0.63037%
===================================================
Files 40 41 +1
Lines 3250 3295 +45
===================================================
+ Hits 1789 1793 +4
- Misses 1324 1362 +38
- Partials 137 140 +3
Continue to review full report at Codecov.
|
mjh1
reviewed
Mar 1, 2023
transcode/transcode.go
Outdated
Comment on lines
115
to
121
| if transcodeRequest.GenerateMP4 { | ||
| for _, profile := range transcodeProfiles { | ||
| renditionList.AddRenditionSegment(profile.Name, | ||
| video.TSegmentList{ | ||
| SegmentDataTable: make(map[int][]byte), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
it could be nice to just late initialise these inside video/media.go when calls are made to GetSegmentList further down
There was a problem hiding this comment.
Since the rest of the usage of the table happens inside concurrent routines, I wanted to keep the logic simplified and easier to debug by pre-creating the empty nested maps (tables) so that we're just getting/inserting in the go-routines.
emranemran
force-pushed
the
emran/concat-ts-segments
branch
3 times, most recently
from
e0bfd82
to
5629834
Compare
… file The segments returned from the T are first stored in the order they are received, and then sorted according to the segment index, and then concatenated into a single .ts file. A nested map with an outer and inner table is used to temporarily store the segment data streams.
emranemran
force-pushed
the
emran/concat-ts-segments
branch
from
5629834
to
b165e0f
Compare
|
Updates:
|
mjh1
approved these changes
Mar 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The segments returned from the T are first stored in the order they are
received, and then sorted according to the segment index, and then
concatenated into a single .ts file. A nested map with an outer and
inner table is used to temporarily store the segment data streams.
TODOs (will use follow-up PR for these):