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

Refactor FFmpegReader GetAVFrame (for AV1 & async decoding) #872

Merged
merged 14 commits into from
Nov 16, 2022

Conversation

jonoomph
Copy link
Member

Some decoders, for example dav1d, process packets asynchronously and are multi-threaded. OpenShot was not correctly handling these async decoders, which resulted in dropping packets and freezing videos. This PR adds an AV1 decoding unit test as a verification, and correctly retries packets that fail to be sent to the decoder.

For example, if a decoder can accept many packets before returning an AVFrame, it might also stop accepting new packets... until it's done processing the previous ones. In those cases, we need to halt getting the next packet, until we can successfully send the current packet.

@jonoomph
Copy link
Member Author

@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #872 (4039851) into develop (854a3aa) will increase coverage by 0.15%.
The diff coverage is 76.23%.

@@             Coverage Diff             @@
##           develop     #872      +/-   ##
===========================================
+ Coverage    51.73%   51.88%   +0.15%     
===========================================
  Files          187      187              
  Lines        16266    16328      +62     
===========================================
+ Hits          8415     8472      +57     
- Misses        7851     7856       +5     
Impacted Files Coverage Δ
src/FFmpegReader.h 100.00% <ø> (ø)
src/FFmpegReader.cpp 73.60% <64.06%> (-0.04%) ⬇️
tests/FFmpegReader.cpp 99.42% <97.29%> (-0.58%) ⬇️
src/Exceptions.h 26.00% <0.00%> (+3.00%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

src/FFmpegReader.cpp Outdated Show resolved Hide resolved
@@ -977,7 +982,7 @@ std::shared_ptr<Frame> FFmpegReader::ReadStream(int64_t requested_frame) {

// Video packet
if ((info.has_video && packet && packet->stream_index == videoStream) ||
(info.has_video && !packet && packet_status.video_decoded < packet_status.video_read) ||
(info.has_video && packet_status.video_decoded < packet_status.video_read) ||
Copy link
Member Author

Choose a reason for hiding this comment

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

Call ProcessVideoPacket() anytime video_decoded < video_read... so we can potentially resend a packet if needed, or try and retrieve a processed AVFrame from the decoder.

@jonoomph jonoomph merged commit 7617e91 into develop Nov 16, 2022
@jonoomph jonoomph deleted the failing-video-decode branch November 16, 2022 23:48
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.

1 participant