-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Attempt to fix FFMPEG 5.0 compatibility #5644
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
Changes from 19 commits
2a05e5f
43922a6
866b30a
d0cadf9
3a927c6
7984483
f9c8987
5e11472
4c6c8fc
08232e4
2f79d6c
d124ee1
df9cb7e
2f7c02a
926a11e
2602ef4
b978ba1
0f02ed9
50d234f
1ffc0ad
f0212c0
41c0ad9
1811fa1
93e0863
3487c68
3c7e998
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,21 +43,34 @@ int SubtitleStream::initFormat() { | |
int SubtitleStream::analyzePacket(const AVPacket* packet, bool* gotFrame) { | ||
// clean-up | ||
releaseSubtitle(); | ||
|
||
// FIXME: should this even be created? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree - I am also not sure if this should be created. Is the plan to follow up on this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I think this would require a bit of coordination on Meta's side, as this is not used nor tested in TV. |
||
AVPacket* avPacket; | ||
avPacket = av_packet_alloc(); | ||
if (avPacket == nullptr) { | ||
LOG(ERROR) | ||
<< "decoder as not able to allocate the subtitle-specific packet."; | ||
// alternative to ENOMEM | ||
return AVERROR_BUFFER_TOO_SMALL; | ||
bjuncek marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we align the error code with the corresponding bit in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jdsgomes The two methods return different enums. Originally Bruno had them synced; no strong opinions on my side. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jdsgomes would you like me to change it to match? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer for consistency, unless there is a reason not to. Other than that the PR looks good and the internal tests are passing. Thank you for these changes |
||
} | ||
avPacket->data = nullptr; | ||
avPacket->size = 0; | ||
datumbox marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// check flush packet | ||
AVPacket avPacket; | ||
av_init_packet(&avPacket); | ||
avPacket.data = nullptr; | ||
avPacket.size = 0; | ||
auto pkt = packet ? *packet : avPacket; | ||
auto pkt = packet ? packet : avPacket; | ||
|
||
int gotFramePtr = 0; | ||
int result = avcodec_decode_subtitle2(codecCtx_, &sub_, &gotFramePtr, &pkt); | ||
// is these a better way than cast from const? | ||
int result = | ||
avcodec_decode_subtitle2(codecCtx_, &sub_, &gotFramePtr, (AVPacket*)pkt); | ||
|
||
if (result < 0) { | ||
LOG(ERROR) << "avcodec_decode_subtitle2 failed, err: " | ||
<< Util::generateErrorDesc(result); | ||
// free the packet we've created | ||
av_packet_free(&avPacket); | ||
datumbox marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return result; | ||
} else if (result == 0) { | ||
result = pkt.size; // discard the rest of the package | ||
result = pkt->size; // discard the rest of the package | ||
} | ||
|
||
sub_.release = gotFramePtr; | ||
|
@@ -66,9 +79,10 @@ int SubtitleStream::analyzePacket(const AVPacket* packet, bool* gotFrame) { | |
// set proper pts in us | ||
if (gotFramePtr) { | ||
sub_.pts = av_rescale_q( | ||
pkt.pts, inputCtx_->streams[format_.stream]->time_base, timeBaseQ); | ||
pkt->pts, inputCtx_->streams[format_.stream]->time_base, timeBaseQ); | ||
} | ||
|
||
av_packet_free(&avPacket); | ||
return result; | ||
bjuncek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.