| Message ID | 20200502131159.28589-1-derek.buitenhuis@gmail.com |
|---|---|
| State | Accepted |
| Commit | 422f1e32ead99a758375c713447b341c8ac3b20d |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/librav1e: Require a bitrate to be set when using 2-pass mode | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On 5/2/2020 10:11 AM, Derek Buitenhuis wrote: > Not requiring this leads to unexpected result, since Rav1e's current > two pass API has no way to fail in such a case. > > Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> > --- > libavcodec/librav1e.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/libavcodec/librav1e.c b/libavcodec/librav1e.c > index b0ff60d8c7..6f9b4cce4c 100644 > --- a/libavcodec/librav1e.c > +++ b/libavcodec/librav1e.c > @@ -202,6 +202,12 @@ static av_cold int librav1e_encode_init(AVCodecContext *avctx) > }); > } > > + if ((avctx->flags & AV_CODEC_FLAG_PASS1 || avctx->flags & AV_CODEC_FLAG_PASS2) && !avctx->bit_rate) { Maybe avctx->flags & (AV_CODEC_FLAG_PASS1 | AV_CODEC_FLAG_PASS2) instead. > + av_log(avctx, AV_LOG_ERROR, "A bitrate must be set to use two pass mode.\n"); > + ret = AVERROR_INVALIDDATA; > + goto end; > + } > + > if (avctx->flags & AV_CODEC_FLAG_PASS2) { > if (!avctx->stats_in) { > av_log(avctx, AV_LOG_ERROR, "No stats file provided for second pass.\n"); > LGTM either way.
On 02/05/2020 15:46, James Almer wrote: >> + if ((avctx->flags & AV_CODEC_FLAG_PASS1 || avctx->flags & AV_CODEC_FLAG_PASS2) && !avctx->bit_rate) { > > Maybe avctx->flags & (AV_CODEC_FLAG_PASS1 | AV_CODEC_FLAG_PASS2) instead. I thought about it, but I think the way I already have it is a little clearer, and so prefer it. >> if (avctx->flags & AV_CODEC_FLAG_PASS2) { >> if (!avctx->stats_in) { >> av_log(avctx, AV_LOG_ERROR, "No stats file provided for second pass.\n"); >> > > LGTM either way. Pushed, thanks. - Derek
diff --git a/libavcodec/librav1e.c b/libavcodec/librav1e.c index b0ff60d8c7..6f9b4cce4c 100644 --- a/libavcodec/librav1e.c +++ b/libavcodec/librav1e.c @@ -202,6 +202,12 @@ static av_cold int librav1e_encode_init(AVCodecContext *avctx) }); } + if ((avctx->flags & AV_CODEC_FLAG_PASS1 || avctx->flags & AV_CODEC_FLAG_PASS2) && !avctx->bit_rate) { + av_log(avctx, AV_LOG_ERROR, "A bitrate must be set to use two pass mode.\n"); + ret = AVERROR_INVALIDDATA; + goto end; + } + if (avctx->flags & AV_CODEC_FLAG_PASS2) { if (!avctx->stats_in) { av_log(avctx, AV_LOG_ERROR, "No stats file provided for second pass.\n");
Not requiring this leads to unexpected result, since Rav1e's current two pass API has no way to fail in such a case. Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> --- libavcodec/librav1e.c | 6 ++++++ 1 file changed, 6 insertions(+)