Skip to content

Feature/muted audio fixes #51 #64

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

Merged
merged 9 commits into from
Mar 4, 2020

Conversation

mudar
Copy link
Collaborator

@mudar mudar commented Mar 1, 2020

Hi Mattia,
This PR solves the problem I was facing when concatenating two dataSources where one would have audio+video tracks and the other would have a video track only (no audio track).
The new MutedAudioDataSource provides a silent sound track, which allows the concatenation to go on correctly.
How would you prefer to do the check that triggers the use of this new DataSource? Initially I thought it could be an AudioStrategy, or a builder option. For now (wip), I've added it in Builder.addDataSource(dataSource) by default, and it can be skipped by using Builder.addDataSource(track, dataSource).
I did many tests with the audio format values (bufferSize, periodSize, periodTime) and these values are the ones that worked for me 💯
thanks :)

Provides a silent audio track of the a specific duration.
This class can be used to concatenate a DataSources that has a video
track only with another that has both video and audio track.
Using addDataSource(dataSource) adds a muted audio track when necessary.
This can be skipped by calling addDataSource(AUDIO, dataSource) and
addDataSource(VIDEO, dataSource).
@natario1
Copy link
Member

natario1 commented Mar 2, 2020

Thanks @mudar this is great! :-) I'll take a look at the source soon. The idea of adding it automatically to the Builder seems very good to me. Let's do this under the hood

@natario1
Copy link
Member

natario1 commented Mar 2, 2020

If you could just find a way to enable this only when needed that'd be great. Maybe at build() time?

I mean now if I pass a single file that is video only, I expect the output to be video only, so we shouldn't add muted audio in this case.

@mudar
Copy link
Collaborator Author

mudar commented Mar 2, 2020

fixed, with the help of @cbernier2 :)
When build() is called, it calls buildAudioDataSources() to check if audio tracks are in a messy situation! The muted audio is added only when needed.

Copy link
Member

@natario1 natario1 left a comment

Choose a reason for hiding this comment

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

Just some small changes, thanks!

mudar added 2 commits March 4, 2020 11:36
and removed unncessary check in canReadTrack()
@mudar mudar force-pushed the feature/muted-audio branch from ba7933e to 573df81 Compare March 4, 2020 20:08
Copy link
Member

@natario1 natario1 left a comment

Choose a reason for hiding this comment

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

Thanks guys this is great. Great reference for a BlankVideo source as well, that seems quite easy now.

This source could be improved in readTrack(), because the last read will always give more data than it should (that is, durationUs is not an exact multiple of the period time). But we can address this later.

@natario1 natario1 merged commit 125d40f into deepmedia:master Mar 4, 2020
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.

2 participants