Message ID | 20220405121539.1773869-1-danilchap@google.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] avcodec/libvpxenc: return quantizer parameter for an encoded frame | expand |
Context | Check | Description |
---|---|---|
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 |
5 Apr 2022, 14:15 by danilchap-at-google.com@ffmpeg.org: > --- > libavcodec/libvpxenc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > index dff1d06b0e..0705863450 100644 > --- a/libavcodec/libvpxenc.c > +++ b/libavcodec/libvpxenc.c > @@ -1250,6 +1250,8 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0); > uint8_t *side_data; > int pict_type; > + int quality; > + VPxContext *ctx = avctx->priv_data; > > if (ret < 0) > return ret; > @@ -1264,7 +1266,12 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > pict_type = AV_PICTURE_TYPE_P; > } > > - ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1, > + ret = vpx_codec_control(&ctx->encoder, VP8E_GET_LAST_QUANTIZER_64, &quality); > + if (ret != VPX_CODEC_OK) { > + quality = 0; > + } > Fix the coding style.
On 4/5/2022 10:55 AM, Lynne wrote: > 5 Apr 2022, 14:15 by danilchap-at-google.com@ffmpeg.org: > >> --- >> libavcodec/libvpxenc.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c >> index dff1d06b0e..0705863450 100644 >> --- a/libavcodec/libvpxenc.c >> +++ b/libavcodec/libvpxenc.c >> @@ -1250,6 +1250,8 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, >> int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0); >> uint8_t *side_data; >> int pict_type; >> + int quality; >> + VPxContext *ctx = avctx->priv_data; >> >> if (ret < 0) >> return ret; >> @@ -1264,7 +1266,12 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, >> pict_type = AV_PICTURE_TYPE_P; >> } >> >> - ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1, >> + ret = vpx_codec_control(&ctx->encoder, VP8E_GET_LAST_QUANTIZER_64, &quality); >> + if (ret != VPX_CODEC_OK) { >> + quality = 0; >> + } >> > > Fix the coding style. You could just say to remove the brackets for one line statements.
On 4/5/2022 9:15 AM, Danil Chapovalov wrote: > --- > libavcodec/libvpxenc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > index dff1d06b0e..0705863450 100644 > --- a/libavcodec/libvpxenc.c > +++ b/libavcodec/libvpxenc.c > @@ -1250,6 +1250,8 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0); > uint8_t *side_data; > int pict_type; > + int quality; > + VPxContext *ctx = avctx->priv_data; > > if (ret < 0) > return ret; > @@ -1264,7 +1266,12 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > pict_type = AV_PICTURE_TYPE_P; > } > > - ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1, > + ret = vpx_codec_control(&ctx->encoder, VP8E_GET_LAST_QUANTIZER_64, &quality); > + if (ret != VPX_CODEC_OK) { Is the last argument guaranteed to be untouched when the return value of vpx_codec_control() is not VPX_CODEC_OK? If so, you can initialize quality to 0 above, and remove this part. > + quality = 0; > + } > + > + ff_side_data_set_encoder_stats(pkt, quality * FF_QP2LAMBDA, cx_frame->sse + 1, > cx_frame->have_sse ? 3 : 0, pict_type); > > if (cx_frame->have_sse) {
I've rechecked documentation for the "vpx_codec_control" - can't find any guarantee about the last parameter when the function fails, thus prefer to be on the safe side. On Tue, Apr 5, 2022 at 3:59 PM James Almer <jamrial@gmail.com> wrote: > > On 4/5/2022 9:15 AM, Danil Chapovalov wrote: > > --- > > libavcodec/libvpxenc.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > > index dff1d06b0e..0705863450 100644 > > --- a/libavcodec/libvpxenc.c > > +++ b/libavcodec/libvpxenc.c > > @@ -1250,6 +1250,8 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > > int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0); > > uint8_t *side_data; > > int pict_type; > > + int quality; > > + VPxContext *ctx = avctx->priv_data; > > > > if (ret < 0) > > return ret; > > @@ -1264,7 +1266,12 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > > pict_type = AV_PICTURE_TYPE_P; > > } > > > > - ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1, > > + ret = vpx_codec_control(&ctx->encoder, VP8E_GET_LAST_QUANTIZER_64, &quality); > > + if (ret != VPX_CODEC_OK) { > > Is the last argument guaranteed to be untouched when the return value of > vpx_codec_control() is not VPX_CODEC_OK? If so, you can initialize > quality to 0 above, and remove this part. > > > + quality = 0; > > + } > > + > > + ff_side_data_set_encoder_stats(pkt, quality * FF_QP2LAMBDA, cx_frame->sse + 1, > > cx_frame->have_sse ? 3 : 0, pict_type); > > > > if (cx_frame->have_sse) { > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Tue, Apr 5, 2022 at 7:23 AM Danil Chapovalov <danilchap-at-google.com@ffmpeg.org> wrote: > > I've rechecked documentation for the "vpx_codec_control" - can't find > any guarantee about the last parameter when the function fails, thus > prefer to be on the safe side. > The check is fine, though the function can only fail due to invalid arguments and won't modify the value in that case. As an aside, try to avoid top posting on this list: https://ffmpeg.org/contact.html#MailingLists > On Tue, Apr 5, 2022 at 3:59 PM James Almer <jamrial@gmail.com> wrote: > > > > On 4/5/2022 9:15 AM, Danil Chapovalov wrote: > > > --- > > > libavcodec/libvpxenc.c | 9 ++++++++- > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > > > index dff1d06b0e..0705863450 100644 > > > --- a/libavcodec/libvpxenc.c > > > +++ b/libavcodec/libvpxenc.c > > > @@ -1250,6 +1250,8 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > > > int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0); > > > uint8_t *side_data; > > > int pict_type; > > > + int quality; > > > + VPxContext *ctx = avctx->priv_data; > > > > > > if (ret < 0) > > > return ret; > > > @@ -1264,7 +1266,12 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, > > > pict_type = AV_PICTURE_TYPE_P; > > > } > > > > > > - ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1, > > > + ret = vpx_codec_control(&ctx->encoder, VP8E_GET_LAST_QUANTIZER_64, &quality); > > > + if (ret != VPX_CODEC_OK) { > > > > Is the last argument guaranteed to be untouched when the return value of > > vpx_codec_control() is not VPX_CODEC_OK? If so, you can initialize > > quality to 0 above, and remove this part. > > > > > + quality = 0; > > > + } > > > + > > > + ff_side_data_set_encoder_stats(pkt, quality * FF_QP2LAMBDA, cx_frame->sse + 1, > > > cx_frame->have_sse ? 3 : 0, pict_type); > > > > > > if (cx_frame->have_sse) { > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c index dff1d06b0e..0705863450 100644 --- a/libavcodec/libvpxenc.c +++ b/libavcodec/libvpxenc.c @@ -1250,6 +1250,8 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, int ret = ff_get_encode_buffer(avctx, pkt, cx_frame->sz, 0); uint8_t *side_data; int pict_type; + int quality; + VPxContext *ctx = avctx->priv_data; if (ret < 0) return ret; @@ -1264,7 +1266,12 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, pict_type = AV_PICTURE_TYPE_P; } - ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1, + ret = vpx_codec_control(&ctx->encoder, VP8E_GET_LAST_QUANTIZER_64, &quality); + if (ret != VPX_CODEC_OK) { + quality = 0; + } + + ff_side_data_set_encoder_stats(pkt, quality * FF_QP2LAMBDA, cx_frame->sse + 1, cx_frame->have_sse ? 3 : 0, pict_type); if (cx_frame->have_sse) {