| Message ID | 20220324131116.825587-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 |
On Thu, Mar 24, 2022 at 6:12 AM Danil Chapovalov <danilchap-at-google.com@ffmpeg.org> wrote: > > --- > libavcodec/libvpxenc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > index dff1d06b0e..463a658bb0 100644 > --- a/libavcodec/libvpxenc.c > +++ b/libavcodec/libvpxenc.c > @@ -1625,6 +1625,12 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > vpx_svc_layer_id_t layer_id; > int layer_id_valid = 0; > > + if (avctx->qmax >= 0 && enccfg->rc_max_quantizer != avctx->qmax) { > + struct vpx_codec_enc_cfg cfg = *enccfg; > + cfg.rc_max_quantizer = avctx->qmax; > + vpx_codec_enc_config_set(&ctx->encoder, &cfg); > + } > + Jan, I think this was what you were suggesting, no? The docs could be updated to note qmax can be changed per-frame [1][2]. Saying that, it does seem a bit unbalanced to only do qmax here. [1] https://ffmpeg.org/ffmpeg-codecs.html#libvpx [2] https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/encoders.texi#l2000
On Thu, Mar 24, 2022 at 7:27 PM James Zern <jzern@google.com> wrote: > > On Thu, Mar 24, 2022 at 6:12 AM Danil Chapovalov > <danilchap-at-google.com@ffmpeg.org> wrote: > > > > --- > > libavcodec/libvpxenc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > > index dff1d06b0e..463a658bb0 100644 > > --- a/libavcodec/libvpxenc.c > > +++ b/libavcodec/libvpxenc.c > > @@ -1625,6 +1625,12 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > > vpx_svc_layer_id_t layer_id; > > int layer_id_valid = 0; > > > > + if (avctx->qmax >= 0 && enccfg->rc_max_quantizer != avctx->qmax) { > > + struct vpx_codec_enc_cfg cfg = *enccfg; > > + cfg.rc_max_quantizer = avctx->qmax; > > + vpx_codec_enc_config_set(&ctx->encoder, &cfg); > > + } > > + > > Jan, I think this was what you were suggesting, no? > The docs could be updated to note qmax can be changed per-frame > [1][2]. Saying that, it does seem a bit unbalanced to only do qmax > here. > > [1] https://ffmpeg.org/ffmpeg-codecs.html#libvpx > [2] https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/encoders.texi#l2000 As I understand, docs describe command line options. I do not plan to expose changing qmax per frame as a command line option, I do not see how that can be reasonably done. My intent is to change max qp when ffmpeg is used as a library. I agree it looks unbalanced to change just the qmax, but that is the only parameter I currently need for my usecase. Personally I prefer to only add features that are planned to be used. Are there any particular configuration settings you want me to make configurable, or do you think it is better to support all settings that can be configurable?
On Mon, Mar 28, 2022 at 7:05 AM Danil Chapovalov <danilchap@google.com> wrote: > > On Thu, Mar 24, 2022 at 7:27 PM James Zern <jzern@google.com> wrote: > > > > On Thu, Mar 24, 2022 at 6:12 AM Danil Chapovalov > > <danilchap-at-google.com@ffmpeg.org> wrote: > > > > > > --- > > > libavcodec/libvpxenc.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > > > index dff1d06b0e..463a658bb0 100644 > > > --- a/libavcodec/libvpxenc.c > > > +++ b/libavcodec/libvpxenc.c > > > @@ -1625,6 +1625,12 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > > > vpx_svc_layer_id_t layer_id; > > > int layer_id_valid = 0; > > > > > > + if (avctx->qmax >= 0 && enccfg->rc_max_quantizer != avctx->qmax) { > > > + struct vpx_codec_enc_cfg cfg = *enccfg; > > > + cfg.rc_max_quantizer = avctx->qmax; > > > + vpx_codec_enc_config_set(&ctx->encoder, &cfg); > > > + } > > > + > > > > Jan, I think this was what you were suggesting, no? > > The docs could be updated to note qmax can be changed per-frame > > [1][2]. Saying that, it does seem a bit unbalanced to only do qmax > > here. > > > > [1] https://ffmpeg.org/ffmpeg-codecs.html#libvpx > > [2] https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/encoders.texi#l2000 > > As I understand, docs describe command line options. > I do not plan to expose changing qmax per frame as a command line > option, I do not see how that can be reasonably done. My intent is to > change max qp when ffmpeg is used as a library. > That is true, I was thinking about it from the library perspective. libx264 doesn't mention this behavior but the codec options section does talk about setting these from the API, so I think a note is worthwhile as this is the only point of documentation for the codec wrapper. > I agree it looks unbalanced to change just the qmax, but that is the > only parameter I currently need for my usecase. Personally I prefer to > only add features that are planned to be used. > Are there any particular configuration settings you want me to make > configurable, > or do you think it is better to support all settings that can be configurable? Since libvpx supports a reconfigure it should be safe to check the options again. There is some complexity in that, though, so we can leave this as a targeted approach similar to libx264 for now unless others have a strong opinion.
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c index dff1d06b0e..463a658bb0 100644 --- a/libavcodec/libvpxenc.c +++ b/libavcodec/libvpxenc.c @@ -1625,6 +1625,12 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, vpx_svc_layer_id_t layer_id; int layer_id_valid = 0; + if (avctx->qmax >= 0 && enccfg->rc_max_quantizer != avctx->qmax) { + struct vpx_codec_enc_cfg cfg = *enccfg; + cfg.rc_max_quantizer = avctx->qmax; + vpx_codec_enc_config_set(&ctx->encoder, &cfg); + } + if (frame) { const AVFrameSideData *sd = av_frame_get_side_data(frame, AV_FRAME_DATA_REGIONS_OF_INTEREST); rawimg = &ctx->rawimg;