| Message ID | 20220316120759.2292546-1-danilchap@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/libvpxenc: enable dynamic max quantizer parameter reconfiguration | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
16 Mar 2022, 13:07 by danilchap-at-google.com@ffmpeg.org: > --- > libavcodec/libvpxenc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > index 8f94ba15dc..45baeed435 100644 > --- a/libavcodec/libvpxenc.c > +++ b/libavcodec/libvpxenc.c > @@ -1658,6 +1658,13 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > flags |= strtoul(en->value, NULL, 10); > } > > + en = av_dict_get(frame->metadata, "max-quantizer", NULL, 0); > + if (en) { > + struct vpx_codec_enc_cfg cfg = *enccfg; > + cfg.rc_max_quantizer = strtoul(en->value, NULL, 10); > + vpx_codec_enc_config_set(&ctx->encoder, &cfg); > + } > I was going to NAK the earlier patch, but forgot. Please think of a better way than stuffing random data in inappropriate fields, and fix the earlier patch that was mistakenly merged to not do that.
Lynne, On Wed, Mar 16, 2022 at 5:39 AM Lynne <dev@lynne.ee> wrote: > > 16 Mar 2022, 13:07 by danilchap-at-google.com@ffmpeg.org: > > > --- > > libavcodec/libvpxenc.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > > index 8f94ba15dc..45baeed435 100644 > > --- a/libavcodec/libvpxenc.c > > +++ b/libavcodec/libvpxenc.c > > @@ -1658,6 +1658,13 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > > flags |= strtoul(en->value, NULL, 10); > > } > > > > + en = av_dict_get(frame->metadata, "max-quantizer", NULL, 0); > > + if (en) { > > + struct vpx_codec_enc_cfg cfg = *enccfg; > > + cfg.rc_max_quantizer = strtoul(en->value, NULL, 10); > > + vpx_codec_enc_config_set(&ctx->encoder, &cfg); > > + } > > > > I was going to NAK the earlier patch, but forgot. > Please think of a better way than stuffing random data in > inappropriate fields, and fix the earlier patch that was mistakenly > merged to not do that. I agree this isn't great. Would you suggest updating the qmax setting and checking it / all options on a per-frame basis or is there a better way to reconfigure the codec on the fly?
On Thu, Mar 17, 2022 at 1:53 AM James Zern <jzern-at-google.com@ffmpeg.org> wrote: > > Lynne, > > > On Wed, Mar 16, 2022 at 5:39 AM Lynne <dev@lynne.ee> wrote: > > > > 16 Mar 2022, 13:07 by danilchap-at-google.com@ffmpeg.org: > > > > > --- > > > libavcodec/libvpxenc.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > > > index 8f94ba15dc..45baeed435 100644 > > > --- a/libavcodec/libvpxenc.c > > > +++ b/libavcodec/libvpxenc.c > > > @@ -1658,6 +1658,13 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > > > flags |= strtoul(en->value, NULL, 10); > > > } > > > > > > + en = av_dict_get(frame->metadata, "max-quantizer", NULL, 0); > > > + if (en) { > > > + struct vpx_codec_enc_cfg cfg = *enccfg; > > > + cfg.rc_max_quantizer = strtoul(en->value, NULL, 10); > > > + vpx_codec_enc_config_set(&ctx->encoder, &cfg); > > > + } > > > > > > > I was going to NAK the earlier patch, but forgot. > > Please think of a better way than stuffing random data in > > inappropriate fields, and fix the earlier patch that was mistakenly > > merged to not do that. > > I agree this isn't great. Would you suggest updating the qmax setting > and checking it / all options on a per-frame basis or is there a > better way to reconfigure the codec on the fly? The libx264 wrapper is probably one of the oldest that supports runtime reconfiguration. Looking at how it does it right now, it seems to call its reconfig_encoder function for each fed non-nullptr AVFrame. So yes, re-reading configuration from the context seems like a viable alternative. Jan
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c index 8f94ba15dc..45baeed435 100644 --- a/libavcodec/libvpxenc.c +++ b/libavcodec/libvpxenc.c @@ -1658,6 +1658,13 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, flags |= strtoul(en->value, NULL, 10); } + en = av_dict_get(frame->metadata, "max-quantizer", NULL, 0); + if (en) { + struct vpx_codec_enc_cfg cfg = *enccfg; + cfg.rc_max_quantizer = strtoul(en->value, NULL, 10); + vpx_codec_enc_config_set(&ctx->encoder, &cfg); + } + memset(&layer_id, 0, sizeof(layer_id)); en = av_dict_get(frame->metadata, "temporal_id", NULL, 0);