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

Update for compatibility with libx265 4.0 #1314

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

Lastique
Copy link
Contributor

libx265 4.0 has changed the signature of x265_api::encoder_encode and x265_encoder_encode functions to accept a pointer to an array of pointers to output pictures. That pointer is now required to be non-null, otherwise the encode call crashes.

Upstream bug report: https://bitbucket.org/multicoreware/x265_git/issues/952/crash-in-libheif-tests

libx265 4.0 has changed the signature of `x265_api::encoder_encode` and
`x265_encoder_encode` functions to accept a pointer to an array of
pointers to output pictures. That pointer is now required to be non-null,
otherwise the encode call crashes.

Upstream bug report: https://bitbucket.org/multicoreware/x265_git/issues/952/crash-in-libheif-tests
@farindk
Copy link
Contributor

farindk commented Sep 17, 2024

Thanks you.

Does that image have to be released?
I'm also wondering whether having the output image affects the encoding speed.

@Lastique
Copy link
Contributor Author

Does that image have to be released?

No. My understanding is that the purpose of the output picture is to communicate to the caller various metadata, such as the encoded frame type (intra vs. inter-dependent), timestamps and so on. The only thing that changed is the number of these pictures that could be populated - in previous libx265 releases it was at most one, in libx265 4.0 it's one picture per layer. The number of layers is specified in x265_param::numLayers and by default is 1.

I'm also wondering whether having the output image affects the encoding speed.

To be clear, this PR still doesn't produce the output picture, as the original code didn't. encoder_encode does not allocate a picture, if the pointer is null. If you want to receive an output image then each pointer to an output image should point to a pre-existing x265_picture struct that encoder_encode will then populate with data.

However, requesting an output picture should not meaningfully affect performance, as it should just populate the struct members. Producing a decoded picture should happen internally anyway to implement the encoder feedback loop. I didn't benchmark, though.

@farindk
Copy link
Contributor

farindk commented Sep 17, 2024

Thanks for the background information.

Producing a decoded picture should happen internally anyway to implement the encoder feedback loop. I didn't benchmark, though.

Yes, but since we're only encoding one I-frame, that reconstructed image is not needed. But I don't believe x265 considers this special case.

@farindk farindk merged commit a659a7c into strukturag:master Sep 17, 2024
35 checks passed
@Lastique Lastique deleted the patch-1 branch September 17, 2024 20:43
@farindk
Copy link
Contributor

farindk commented Oct 30, 2024

x265 changed the function signature back to the old parameters in build version 213. I have adapted the #if in 9c39813.

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