Message ID | 20181025125858.31807-2-martin@martin.st |
---|---|
State | Accepted |
Headers | show |
On 25/10/2018 13:58, Martin Storsjö wrote:
> + x4->nb_reordered_opaque = x264_encoder_maximum_delayed_frames(x4->enc) + 1;
Is it possible this changes when the encoder is reconfigured (e.g. to interlaced)?
- Derek
On Mon, 29 Oct 2018, Derek Buitenhuis wrote: > On 25/10/2018 13:58, Martin Storsjö wrote: >> + x4->nb_reordered_opaque = x264_encoder_maximum_delayed_frames(x4->enc) + 1; > > Is it possible this changes when the encoder is reconfigured (e.g. to interlaced)? Good point. I'm sure it's possible that it changes, if reconfiguring. As I guess there can be old frames in flight, the only safe option is to enlarge, not to shrink it. But in case a realloc moves the array, the old pointers end up pretty useless. Tricky, I guess I'll have to think about it to see if I can come up with something which isn't completely terrible. // Martin
On 29/10/2018 21:06, Martin Storsjö wrote: > As I guess there can be old frames in flight, the only safe option is to > enlarge, not to shrink it. But in case a realloc moves the array, the old > pointers end up pretty useless. Just always allocate the max (which is known for H.264), and adjust nb_reordered_opaque as need be, on reconfig, no? - Derek
On Tue, 30 Oct 2018, Derek Buitenhuis wrote: > On 29/10/2018 21:06, Martin Storsjö wrote: >> As I guess there can be old frames in flight, the only safe option is to >> enlarge, not to shrink it. But in case a realloc moves the array, the old >> pointers end up pretty useless. > > Just always allocate the max (which is known for H.264), and adjust nb_reordered_opaque > as need be, on reconfig, no? Hmm, that might make sense, but with a little twist. The max reordered frames for H.264 is known, but onto that you also get more delay due to frame threads and other details that this function within x264 knows about. So that would make it [H264 max reordering] + [threads] + [constant] or something such? // Martin
On 30/10/2018 19:49, Martin Storsjö wrote: > Hmm, that might make sense, but with a little twist. The max reordered > frames for H.264 is known, but onto that you also get more delay due to > frame threads and other details that this function within x264 knows > about. So that would make it [H264 max reordering] + [threads] + > [constant] or something such? Looking at the source, it's more complicated than that, with e.g.: h->frames.i_delay = X264_MAX( h->frames.i_delay, h->param.rc.i_lookahead ); I think you're better off not trying to duplicate this logic. - Derek
On Wed, 31 Oct 2018, Derek Buitenhuis wrote: > On 30/10/2018 19:49, Martin Storsjö wrote: >> Hmm, that might make sense, but with a little twist. The max reordered >> frames for H.264 is known, but onto that you also get more delay due to >> frame threads and other details that this function within x264 knows >> about. So that would make it [H264 max reordering] + [threads] + >> [constant] or something such? > > Looking at the source, it's more complicated than that, with e.g.: > > h->frames.i_delay = X264_MAX( h->frames.i_delay, h->param.rc.i_lookahead ); > > I think you're better off not trying to duplicate this logic. Indeed, I don't want to duplicate that. Even though we do allow reconfiguration, it doesn't look like we support changing any parameters which would actually affect the delay, only RC method and targets (CRF, bitrate, etc). So given that, the current patch probably should be safe - what do you think? Or the current patch, with an added margin of 16 on top just in case? // Martin
On 31/10/2018 21:41, Martin Storsjö wrote: > Even though we do allow reconfiguration, it doesn't look like we support > changing any parameters which would actually affect the delay, only RC > method and targets (CRF, bitrate, etc). So given that, the current patch > probably should be safe - what do you think? Or the current patch, with an > added margin of 16 on top just in case? We allow reconfiguring to/from interlaced. I'm not sure if this can modify delay? Othwewise, probably safe, I guess. - Derek
On Thu, 1 Nov 2018, Derek Buitenhuis wrote: > On 31/10/2018 21:41, Martin Storsjö wrote: >> Even though we do allow reconfiguration, it doesn't look like we support >> changing any parameters which would actually affect the delay, only RC >> method and targets (CRF, bitrate, etc). So given that, the current patch >> probably should be safe - what do you think? Or the current patch, with an >> added margin of 16 on top just in case? > > We allow reconfiguring to/from interlaced. I'm not sure if this can modify delay? Not really sure either... So perhaps it'd be safest with some bit of extra margin/overestimate of the delay here? It just costs a couple bytes in the mapping array anyway. // Martin
On Thu, 1 Nov 2018, Martin Storsjö wrote: > On Thu, 1 Nov 2018, Derek Buitenhuis wrote: > >> On 31/10/2018 21:41, Martin Storsjö wrote: >>> Even though we do allow reconfiguration, it doesn't look like we support >>> changing any parameters which would actually affect the delay, only RC >>> method and targets (CRF, bitrate, etc). So given that, the current patch >>> probably should be safe - what do you think? Or the current patch, with an >>> added margin of 16 on top just in case? >> >> We allow reconfiguring to/from interlaced. I'm not sure if this can modify > delay? > > Not really sure either... So perhaps it'd be safest with some bit of extra > margin/overestimate of the delay here? It just costs a couple bytes in the > mapping array anyway. Pushed, with a bit over overestimation of the buffer size, just in case. // Martin
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 705a3ce4f3..69c5a82757 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -1070,6 +1070,13 @@ typedef struct RcOverride{ */ #define AV_CODEC_CAP_HYBRID (1 << 19) +/** + * This codec takes the reordered_opaque field from input AVFrames + * and returns it in the corresponding field in AVCodecContext after + * encoding. + */ +#define AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE (1 << 20) + /** * Pan Scan area. * This specifies the area which should be displayed. @@ -2676,7 +2683,10 @@ typedef struct AVCodecContext { /** * opaque 64-bit number (generally a PTS) that will be reordered and * output in AVFrame.reordered_opaque - * - encoding: unused + * - encoding: Set by libavcodec to the reordered_opaque of the input + * frame corresponding to the last returned packet. Only + * supported by encoders with the + * AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE capability. * - decoding: Set by user. */ int64_t reordered_opaque; diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index d6367bf557..13c86a9451 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -92,6 +92,9 @@ typedef struct X264Context { int noise_reduction; char *x264_params; + + int nb_reordered_opaque, next_reordered_opaque; + int64_t *reordered_opaque; } X264Context; static void X264_log(void *p, int level, const char *fmt, va_list args) @@ -278,6 +281,7 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame, int nnal, i, ret; x264_picture_t pic_out = {0}; int pict_type; + int64_t *out_opaque; x264_picture_init( &x4->pic ); x4->pic.img.i_csp = x4->params.i_csp; @@ -297,6 +301,11 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame, x4->pic.i_pts = frame->pts; + x4->reordered_opaque[x4->next_reordered_opaque] = frame->reordered_opaque; + x4->pic.opaque = &x4->reordered_opaque[x4->next_reordered_opaque]; + x4->next_reordered_opaque++; + x4->next_reordered_opaque %= x4->nb_reordered_opaque; + switch (frame->pict_type) { case AV_PICTURE_TYPE_I: x4->pic.i_type = x4->forced_idr > 0 ? X264_TYPE_IDR @@ -350,6 +359,14 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, const AVFrame *frame, pkt->pts = pic_out.i_pts; pkt->dts = pic_out.i_dts; + out_opaque = pic_out.opaque; + if (out_opaque >= x4->reordered_opaque && + out_opaque < &x4->reordered_opaque[x4->nb_reordered_opaque]) { + ctx->reordered_opaque = *out_opaque; + } else { + // Unexpected opaque pointer on picture output + ctx->reordered_opaque = 0; + } switch (pic_out.i_type) { case X264_TYPE_IDR: @@ -393,6 +410,7 @@ static av_cold int X264_close(AVCodecContext *avctx) av_freep(&avctx->extradata); av_freep(&x4->sei); + av_freep(&x4->reordered_opaque); if (x4->enc) { x264_encoder_close(x4->enc); @@ -846,6 +864,12 @@ FF_ENABLE_DEPRECATION_WARNINGS cpb_props->max_bitrate = x4->params.rc.i_vbv_max_bitrate * 1000; cpb_props->avg_bitrate = x4->params.rc.i_bitrate * 1000; + x4->nb_reordered_opaque = x264_encoder_maximum_delayed_frames(x4->enc) + 1; + x4->reordered_opaque = av_malloc_array(x4->nb_reordered_opaque, + sizeof(*x4->reordered_opaque)); + if (!x4->reordered_opaque) + return AVERROR(ENOMEM); + return 0; } @@ -1059,7 +1083,8 @@ AVCodec ff_libx264_encoder = { .init = X264_init, .encode2 = X264_frame, .close = X264_close, - .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, .priv_class = &x264_class, .defaults = x264_defaults, .init_static_data = X264_init_static, @@ -1085,7 +1110,8 @@ AVCodec ff_libx264rgb_encoder = { .init = X264_init, .encode2 = X264_frame, .close = X264_close, - .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, .priv_class = &rgbclass, .defaults = x264_defaults, .pix_fmts = pix_fmts_8bit_rgb, @@ -1110,7 +1136,8 @@ AVCodec ff_libx262_encoder = { .init = X264_init, .encode2 = X264_frame, .close = X264_close, - .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, .priv_class = &X262_class, .defaults = x264_defaults, .pix_fmts = pix_fmts_8bit, diff --git a/libavcodec/version.h b/libavcodec/version.h index 9098882f47..91809641b4 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -28,8 +28,8 @@ #include "libavutil/version.h" #define LIBAVCODEC_VERSION_MAJOR 58 -#define LIBAVCODEC_VERSION_MINOR 33 -#define LIBAVCODEC_VERSION_MICRO 102 +#define LIBAVCODEC_VERSION_MINOR 34 +#define LIBAVCODEC_VERSION_MICRO 100 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \