| Message ID | 20200519185525.713-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 74dfc88b638f0c3768aee090054bb2fc41e8ae04 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/libx265: add support for reordered_opaque | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On 19/05/2020 19:55, James Almer wrote:
> + if (pic->reordered_opaque) {
If this is meant to be PTS, won't this break on PTS==0?
- Derek
On Tue, May 19, 2020 at 10:45 PM Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote: > > On 19/05/2020 19:55, James Almer wrote: > > + if (pic->reordered_opaque) { > > If this is meant to be PTS, won't this break on PTS==0? > Well, it won't break anything, since if its not set on the frame, it'll be set to zero when the frame returns However, the default value of reordered_opaque is AV_NOPTS_VALUE, so if you want to skip writing one value, it should probably be that one, instead of zero. - Hendrik
On 5/19/2020 5:18 PM, Derek Buitenhuis wrote: > On 19/05/2020 19:55, James Almer wrote: >> + if (pic->reordered_opaque) { > > If this is meant to be PTS, won't this break on PTS==0? > > - Derek When pic->reordered_opaque is 0 (default value) x265pic.userData will remain NULL. This in turn means x265pic_out.userData will also be NULL and avctx->reordered_opaque will be set to 0, so it's functionally the expected behavior. This is done to avoid a malloc() per frame in 99% of use cases where reordered_opaque is not going to be used.
On 19/05/2020 22:03, James Almer wrote: > When pic->reordered_opaque is 0 (default value) x265pic.userData will > remain NULL. This in turn means x265pic_out.userData will also be NULL > and avctx->reordered_opaque will be set to 0, so it's functionally the > expected behavior. > This is done to avoid a malloc() per frame in 99% of use cases where > reordered_opaque is not going to be used. OK, seems fine then. - Derek
On 5/19/2020 5:53 PM, Hendrik Leppkes wrote: > On Tue, May 19, 2020 at 10:45 PM Derek Buitenhuis > <derek.buitenhuis@gmail.com> wrote: >> >> On 19/05/2020 19:55, James Almer wrote: >>> + if (pic->reordered_opaque) { >> >> If this is meant to be PTS, won't this break on PTS==0? >> > > Well, it won't break anything, since if its not set on the frame, > it'll be set to zero when the frame returns > > However, the default value of reordered_opaque is AV_NOPTS_VALUE, so > if you want to skip writing one value, it should probably be that one, > instead of zero. No, the AVCodecContext field and the AVFrame field have different default values. By writing 0 in AVCodecContext in this scenario I'm effectively passing the AVFrame value through, which is the expected behavior for AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE encoders as per the doxy.
On 5/19/2020 6:40 PM, Derek Buitenhuis wrote: > On 19/05/2020 22:03, James Almer wrote: >> When pic->reordered_opaque is 0 (default value) x265pic.userData will >> remain NULL. This in turn means x265pic_out.userData will also be NULL >> and avctx->reordered_opaque will be set to 0, so it's functionally the >> expected behavior. >> This is done to avoid a malloc() per frame in 99% of use cases where >> reordered_opaque is not going to be used. > > OK, seems fine then. > > - Derek Pushed, thanks.
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c index 573ecc8cb0..821175c1b6 100644 --- a/libavcodec/libx265.c +++ b/libavcodec/libx265.c @@ -504,6 +504,16 @@ static int libx265_encode_frame(AVCodecContext *avctx, AVPacket *pkt, ret = libx265_encode_set_roi(ctx, pic, &x265pic); if (ret < 0) return ret; + + if (pic->reordered_opaque) { + x265pic.userData = av_malloc(sizeof(pic->reordered_opaque)); + if (!x265pic.userData) { + av_freep(&x265pic.quantOffsets); + return AVERROR(ENOMEM); + } + + memcpy(x265pic.userData, &pic->reordered_opaque, sizeof(pic->reordered_opaque)); + } } ret = ctx->api->encoder_encode(ctx->encoder, &nal, &nnal, @@ -570,6 +580,12 @@ FF_ENABLE_DEPRECATION_WARNINGS ff_side_data_set_encoder_stats(pkt, x265pic_out.frameData.qp * FF_QP2LAMBDA, NULL, 0, pict_type); + if (x265pic_out.userData) { + memcpy(&avctx->reordered_opaque, x265pic_out.userData, sizeof(avctx->reordered_opaque)); + av_freep(&x265pic_out.userData); + } else + avctx->reordered_opaque = 0; + *got_packet = 1; return 0; } @@ -683,6 +699,7 @@ AVCodec ff_libx265_encoder = { .priv_data_size = sizeof(libx265Context), .priv_class = &class, .defaults = x265_defaults, - .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS, + .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS | + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE, .wrapper_name = "libx265", };
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/libx265.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)