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

Improved AV Reading/Writing #57

Merged
merged 27 commits into from Jan 22, 2021
Merged

Improved AV Reading/Writing #57

merged 27 commits into from Jan 22, 2021

Conversation

IsaMorphic
Copy link
Contributor

This pull request aims to get FFMediaToolkit a few steps closer to a solution for #33.

A couple months ago @kskalski started an implementation of audio stream reading (#48). However, they did leave a few notes about key missing features of the implementation. The most prevalent of these were proper handling of interleaved audio/video data and write support for audio streams.

After some brief deliberation and research about the ffmpeg api, I am ready to begin an implementation of both of these features.

One other feature I will also consider adding is support for multiple audio streams, as I believe it is pretty limiting to allow for either one video stream, one audio stream, or both, but not more than one each.

I will post updates to this thread as I make progress.

@IsaMorphic
Copy link
Contributor Author

Just committed some more work on the implementation. Audio and video streams now share a base "MediaStream" class which allows for less copying and pasting of code between the two.

I have tested the new changes by creating an application that converts a given "test.mp4" that contains one video and one audio stream into a PNG sequence accompanied by a WAV file with all the audio.

It does so by reading from both data streams simultaneously, essentially reading video frames until some audio packets are buffered, and then consuming the buffered data exhaustively. It repeats this process until both streams have no more packets.

Next steps include adapting the MediaFile class to allow for accessing multiple audio and video streams, and of course writing an arbitrary number of audio and video streams to an output file.

@IsaMorphic
Copy link
Contributor Author

Hello, thanks for checking the code out!
I will resolve all of these conversations once I have completed the requested changes.
Cheers!

FFMediaToolkit/Decoding/MediaFile.cs Outdated Show resolved Hide resolved
FFMediaToolkit/Decoding/Internal/InputContainer.cs Outdated Show resolved Hide resolved
FFMediaToolkit/Common/MediaType.cs Show resolved Hide resolved
@IsaMorphic
Copy link
Contributor Author

IsaMorphic commented Jan 13, 2021

I have just finished writing a decent multi-stream audio/video writing implementation for this library. There are some goofy quirks that arise when trying to transcode audio streams with different frame sizes, but frankly that is beyond the scope of this library, and its nothing that a clever user wouldn't be able to abstract out for themselves.

I also realized that audio streams do not always give/receive their data in the form of floats, so I added the proper conversion code so that this ugliness is hidden from the user of the library.

By and by, the code now fits my needs and is ready to be merged into the main repository whenever a maintainer sees it fit.
Cheers!

@IsaMorphic IsaMorphic changed the title Improved AV Reading/Writing (WIP) Improved AV Reading/Writing Jan 13, 2021
Copy link
Contributor

@kskalski kskalski left a comment

Choose a reason for hiding this comment

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

The change looks quite good to me, just some comments about providing old api for compatibility

FFMediaToolkit/Encoding/MediaOutput.cs Outdated Show resolved Hide resolved
FFMediaToolkit/Encoding/MediaOutput.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@kskalski kskalski left a comment

Choose a reason for hiding this comment

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

I think those changes look good

@radek-k radek-k merged commit 1451712 into radek-k:develop Jan 22, 2021
@IsaMorphic IsaMorphic deleted the develop branch February 12, 2021 17:14
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.

None yet

5 participants