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

fix: Fire trackinfo for audio only transmuxers #277

Merged
merged 1 commit into from
Jun 25, 2019

Conversation

brandonocasey
Copy link
Contributor

Right now we only fire trackinfo for tsPipelines, but we should fire it for aacPipelines as well.

@@ -197,9 +193,9 @@ var aacPipeline = function(options) {
return;
}

pipeline.tracks.audio = {
pipeline.tracks.audio = pipeline.tracks.audio || {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this more like the non-partial transmuxer.

@@ -208,19 +204,13 @@ var aacPipeline = function(options) {
// hook up the audio segment stream to the first track with aac data
pipeline.audioSegmentStream = new AudioSegmentStream(pipeline.tracks.audio, options);

pipeline.audioSegmentStream.on('timingInfo',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were firing audioTimingInfo twice, woops

pipeline.trigger.bind(pipeline, 'audioTimingInfo'));

// Set up the final part of the audio pipeline
pipeline.adtsStream
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move the actual pipe below listener definitions just in case.

@@ -266,6 +264,7 @@ var Transmuxer = function(options) {
hasFlushed = true;

Transmuxer.prototype.init.call(this);
options.baseMediaDecodeTime = options.baseMediaDecodeTime || 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this baseMediaDecodeTime defaults to undefined, which is weird.

@brandonocasey brandonocasey force-pushed the fix/audio-only-trackinfo branch from 2e3cc96 to 8cec3df Compare June 21, 2019 21:20
@brandonocasey brandonocasey changed the base branch from feat/parse-mp4-tracks to master June 21, 2019 21:23
@brandonocasey brandonocasey force-pushed the fix/audio-only-trackinfo branch from 8cec3df to 7f7950d Compare June 24, 2019 17:29
@brandonocasey brandonocasey force-pushed the fix/audio-only-trackinfo branch from 7f7950d to 9b917e0 Compare June 24, 2019 18:48
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Going to test the changes now but code makes sense to me.

@gkatsev gkatsev added the tested label Jun 24, 2019
@gkatsev gkatsev merged commit 2d38f7a into master Jun 25, 2019
@gkatsev gkatsev deleted the fix/audio-only-trackinfo branch June 25, 2019 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants