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

Add AVI extraction support #9915

Merged
merged 73 commits into from
Jun 15, 2022
Merged

Add AVI extraction support #9915

merged 73 commits into from
Jun 15, 2022

Conversation

dburckh
Copy link

@dburckh dburckh commented Jan 31, 2022

This PR contains support for the AVI container as defined here:

https://docs.microsoft.com/en-us/windows/win32/directshow/avi-riff-file-reference

As a bonus it also supports adds a Renderer with MJPEG support. The Render is based on BitmapFactory, so it could be easily extended to MPNG or MHEIF, etc.

Video Codecs supported are MJPEG, AVC, MP4V, MP43. MP42 is theoretically supported.
Audio Codecs supported are PCM, MP3, AAC, AC3 and DTS

The primary goal of this PR is support video from older and lower end video cameras. Some examples of devices that use AVI are PowerShot cameras, low end drones and dash cameras. The are predominately MJPEG/PCM.

Further, the MJPEG Renderer could be useful as most webcams support MJPEG.

@AquilesCanta
Copy link
Contributor

I will look into merging this in roughly 3 weeks, as I'm starting my holidays. Thanks again for your contribution.

@dburckh
Copy link
Author

dburckh commented Feb 17, 2022

I will look into merging this in roughly 3 weeks, as I'm starting my holidays. Thanks again for your contribution.

Thank you for your time and effort. Enjoy your holiday!

@dburckh
Copy link
Author

dburckh commented Mar 16, 2022

@AquilesCanta How was holiday? Are we ready to merge this?

I've been building my app with Exo from source. Studio does not like that. :)

@AquilesCanta
Copy link
Contributor

AquilesCanta commented Mar 16, 2022

I've started looking into this. Unfortunately, the internal infrastructure is having some inconveniences because of the AndroidX media3 work. I'll try to do this quickly, but we'll need to be patient. I'll post an update here once this is done, but please don't count on this being merged for a particular release, because we haven't even started the internal review process.

@zhoushsong
Copy link

zhoushsong commented Apr 9, 2022

Thank you very much for sharing. I compiled and tested your code and found that two avi format videos could not be played. I hope you can fix this bug
http://www.melodvb.com/testVideo/1280.avi
http://www.melodvb.com/testVideo/67289.avi

@dburckh
Copy link
Author

dburckh commented Apr 10, 2022

Thank you very much for sharing. I compiled and tested your code and found that two avi format videos could not be played. I hope you can fix this bug http://www.melodvb.com/testVideo/1280.avi http://www.melodvb.com/testVideo/67289.avi

For future reference, please open any issues in my fork. https://github.com/dburckh/ExoPlayer/issues

The first link contained an MP42 aka MPEG-4 Visual (Microsoft) Video codec. It's highly unlikely your device has a codec for it. FYI: I am aware VLC will play those, but VLC has it's own proprietary CODECs that are not generally available to ExoPlayer. The second link was a zero length file. If you will like to discuss this further please open an issue with the link above.

@AquilesCanta
Copy link
Contributor

Thank you all for your interest and patience. We are still working on this.

Just a quick heads up, I've already internally imported your PR some weeks ago, so please don't make any modifications to the PR because those won't be picked up. Please keep the fixes around so that we can incorporate them later, once the initial version has been pushed to Github.

@AquilesCanta
Copy link
Contributor

I'm still working on this pull request. The reason it's taking so long is that the implementation was not I/O error resilient, so I had to restructure it pretty much completely in order to make the extractor able to resume after an IOException or an interruption, which was not the case before. What would help at this stage is if folks could provide sample media using different container formats for me to test the new implementation. The only sample I have right now uses AVC + mpeg/audio. Thanks for your patience.

@dburckh
Copy link
Author

dburckh commented May 18, 2022

H264 is not a typical AVI. I almost didn't support it. There are a lot of problems with AVI because of out of sequence (B) frames. Most streams are XVID/H263. Let me know where you want the samples. I have a ton of them.

My use for AVI is read from a local file system, so IOExceptions are extremely rare. Curious to see what issues you had with interruption and what you had to do to fix them. If you have any questions about the format feel free to ping me.

@AquilesCanta
Copy link
Contributor

What I need is as few samples as possible that cover as much as possible. So, at least one occurrence per compression format among all samples. A sample where the seek offsets start from the start of the file.

Curious to see what issues you had with interruption

Actually, ExoPlayer uses interruptions to cancel loads, but I'm not 100% sure it can reuse extractors after a cancel so in practice I'm not sure interruptions are relevant. But IO Exceptions are a real problem. For instance, you can run out of network access at any point, which will be surfaced as an IO Exception. This is well covered by our ExtractorTest infrastructure, which simulates IO Exceptions for every byte in the stream.

The concrete consequences of this when writing extractor code is that you cannot have an ExtractorInput.readFully (or skip) followed by another, where the first is not protected by the state of the extractor. So for instance:

void readSample() {
  extractorInput.readFully(the header);
  extractorInput.readFully(the body);
}

Is problematic. If an exception occurs reading the body, a resumption of the extraction (which means extractor.read() will be called again) will make the extractor try to read the header again, but using the read position from before, that's pointing at the body. So we use states for that reason. If any read() or skip() fails, the extractor should be able to continue normally next time read() is called.

Please kindly share the samples, and thanks again for the contribution.

@dburckh
Copy link
Author

dburckh commented May 26, 2022

Sorry, was travelling last week. Here are some links to bugs. They have samples.

This one is probably the worst of them H264 with a hacky encoding to simulate variable frame rates. It also has VBR MP3.
dburckh#11

This one is XVID(H263) with MP3 and an incorrect encoding variant (I frames are flagged as uncompressed video).
dburckh#5 (comment)

This one is MJPEG with PCM audio, unfortunately we pulled the software MJPEG codec out of this PR, so the only thing I've found that with a hardware mjpeg codec it is a Nvidia Shield TV.
DV000101.zip

@AquilesCanta
Copy link
Contributor

The PR has been merged internally, so it will be available soon. I've stripped many features to get it merged, in favor of letting the community contribute those features. Please kindly send future PRs to https://github.com/androidx/media, please keep PRs minimally scoped, and please include an AviExtractorTest test (using a minimal sample) that covers the feature that you are sending our way. Our testing infrastructure tests a few essential aspects of the solution for you, like IO Error resilience.

Once the internal change reaches this repository, this PR should be closed automatically. We are grateful for your contribution!

@marcbaechinger marcbaechinger merged commit 1ca382d into google:dev-v2 Jun 15, 2022
marcbaechinger added a commit that referenced this pull request Jun 15, 2022
PiperOrigin-RevId: 455094147
(cherry picked from commit 1ca382d)
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

library/common/src/main/java/com/google/android/exoplayer2/util/FileTypes.java

@dburckh
Copy link
Author

dburckh commented Jun 23, 2022

I built what got merged (release-v2) and it's failing on most of my samples. It appears most of the clock/AV sync code was removed. This is what needs to get added back in (in order) to get a functional release:

  1. The clock code. Since AVI does not store PTS for each frame, it needs to be generated via some sort of clock generator.
  2. The XXX ChunkHandler, this code feeds the clock and keeps it in sync with the stream. Usually you will need one of these per codec. Probably the most import are MP4V and MpegAudio.
  3. The Audio Stream positioning in the SeekMap. The audio stream is muxed in chunks (groups of frames), not frames. The audio frame number is stored for each video frame that can be seeked to sync audio clock. I'll admit that my impl. is hard to follow, but it's hard to store all that info efficiently. Note: Some streams (MJPEG) are 100% key frames, so just storing the key frames can be problematic.

It would be nice to have back the code that fixes up the aspect ratio based on the underlying codec data. A lot of the AVI muxers don't pick up on the anamorphic flags in the video stream when muxing. This results in the video being played back with the wrong aspect ratio.

@AquilesCanta
Copy link
Contributor

This is what needs to get added back in (in order) to get a functional release

Right. We will need to rely on PRs to get that functionality back. Ideally sent to the androidx repo, being self contained (one key functionality per PR), including one test per feature. This will guarantee that future PRs conform to the Extractor's interface requirement around IOException resilience, and that reviews are done quickly.

@dburckh
Copy link
Author

dburckh commented Jun 23, 2022

Unfortunately, I won't have time to work on this until September(?) and that's assuming I don't get a real job. :P I'll leave my fork available for reference. Maybe some other motivated individual can pick up some of the work?

@AquilesCanta
Copy link
Contributor

Hopefully. Thanks again for your contributions!

@AquilesCanta AquilesCanta changed the title AVI Extractor and MJPEG Renderer Add AVI extraction support Jul 5, 2022
@AquilesCanta AquilesCanta mentioned this pull request Jul 5, 2022
rohitjoins pushed a commit that referenced this pull request Jul 7, 2022
PiperOrigin-RevId: 455094147
(cherry picked from commit ad3348c)
rohitjoins pushed a commit that referenced this pull request Jul 13, 2022
PiperOrigin-RevId: 455094147
@google google locked and limited conversation to collaborators Aug 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants