-
Notifications
You must be signed in to change notification settings - Fork 213
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
Feat: change remux after initial creation #276
Conversation
6fb5cf3
to
6848693
Compare
@@ -352,6 +352,15 @@ var Transmuxer = function(options) { | |||
} | |||
}; | |||
|
|||
this.setRemux = function(val) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently this won't do anything as we don't have a coalesceStream
for the partial transmuxer. This also means fmp4 and ts content will not work with the partial transmuxer right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it was intentional that there is no coalesceStream for the partial transmuxer in order to reduce the time to return data
6848693
to
e4d6345
Compare
e4d6345
to
38b8f49
Compare
38b8f49
to
ca7e1c3
Compare
this.pendingBytes += output.boxes.byteLength; | ||
|
||
// TODO: is there an issue for this against chrome? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what caused the test change, and a lot of the headache I was seeing getting this to work
@@ -21,8 +21,8 @@ var AdtsStream = require('../codecs/adts.js'); | |||
var H264Stream = require('../codecs/h264').H264Stream; | |||
var AacStream = require('../aac'); | |||
var isLikelyAacData = require('../aac/utils').isLikelyAacData; | |||
var ONE_SECOND_IN_TS = require('../utils/clock').ONE_SECOND_IN_TS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a required change but we use it this way in most other files. I think I missed this during the original rebase.
@@ -4,6 +4,7 @@ var Stream = require('../utils/stream.js'); | |||
var mp4 = require('../mp4/mp4-generator.js'); | |||
var audioFrameUtils = require('../mp4/audio-frame-utils'); | |||
var trackInfo = require('../mp4/track-decode-info.js'); | |||
var ONE_SECOND_IN_TS = require('../utils/clock').ONE_SECOND_IN_TS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a required change but we use it this way in most other files. I think I missed this during the original rebase.
@@ -3738,7 +3738,7 @@ validateTrackFragment = function(track, segment, metadata, type) { | |||
QUnit.test('parses an example mp2t file and generates combined media segments', function() { | |||
var | |||
segments = [], | |||
i, j, boxes, mfhd, trackType = 'video', trackId = 256, baseOffset = 0, initSegment; | |||
i, j, boxes, mfhd, trackType = 'audio', trackId = 257, baseOffset = 0, initSegment; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we now always output video first, this test had to be changed.
@@ -1119,6 +1129,16 @@ Transmuxer = function(options) { | |||
} | |||
}; | |||
|
|||
this.setRemux = function(val) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This allows us to set the remux
option after the transmuxer is created.
For fmp4 playlists with ts segment discontinuities the track ids of the fmp4 files and the ts segments that we transmux to fmp4 must match up.