| Message ID | 20230907020645.37017-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/av1dec: export pixel format even if no hardware decoder is present | expand |
James Almer: > And remove the AVOID_PROBING flag, given it's the last av1 decoder to be tested > either way. > This fixes a regression introduced in 1652f2492f88434010053289d946dab6a57e4d58, > where even if forcing the native av1 decoder, if another decoder was present, > like libdav1d or libaom-av1, they'd be used for probing and some fate tests > would have different results. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/av1dec.c | 8 ++++---- > tests/fate/lavf-container.mak | 8 ++++---- > tests/ref/fate/av1-annexb-demux | 2 +- > tests/ref/lavf-fate/av1.mkv | 4 ++-- > tests/ref/lavf-fate/av1.mp4 | 4 ++-- > 5 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c > index ec8401f4e0..a1e08185a7 100644 > --- a/libavcodec/av1dec.c > +++ b/libavcodec/av1dec.c > @@ -612,6 +612,9 @@ static int get_pixel_format(AVCodecContext *avctx) > if (ret < 0) > return ret; > > + s->pix_fmt = pix_fmt; > + avctx->pix_fmt = ret; > + > /** > * check if the HW accel is inited correctly. If not, return un-implemented. > * Since now the av1 decoder doesn't support native decode, if it will be > @@ -623,9 +626,6 @@ static int get_pixel_format(AVCodecContext *avctx) > return AVERROR(ENOSYS); Is the log message here actually accurate? The get_format callback choosing the software pixel format does not mean that the hardware this is run on or the lavc binary in use do not support hardware accelerated AV1 decoding. (How is an API user actually supposed to know that decoding will fail if the software pixel format is selected?) > } > > - s->pix_fmt = pix_fmt; > - avctx->pix_fmt = ret; > - > return 0; > } > > @@ -1518,7 +1518,7 @@ const FFCodec ff_av1_decoder = { > .init = av1_decode_init, > .close = av1_decode_free, > FF_CODEC_RECEIVE_FRAME_CB(av1_receive_frame), > - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_AVOID_PROBING, > + .p.capabilities = AV_CODEC_CAP_DR1, > .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > .flush = av1_decode_flush, > .p.profiles = NULL_IF_CONFIG_SMALL(ff_av1_profiles), > diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak > index 0d4a224601..0081b45eea 100644 > --- a/tests/fate/lavf-container.mak > +++ b/tests/fate/lavf-container.mak > @@ -70,9 +70,9 @@ fate-lavf-wtv: CMD = lavf_container "" "-c:a mp2 -threads 1" > FATE_AVCONV += $(FATE_LAVF_CONTAINER) > fate-lavf-container fate-lavf: $(FATE_LAVF_CONTAINER) > > -FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_PARSER MOV_MUXER) += av1.mp4 > +FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_DECODER AV1_PARSER MOV_MUXER) += av1.mp4 > +FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_DECODER AV1_PARSER MATROSKA_MUXER) += av1.mkv > FATE_LAVF_CONTAINER_FATE-$(call ALLYES, EVC_DEMUXER EVC_PARSER MOV_MUXER) += evc.mp4 > -FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_PARSER MATROSKA_MUXER) += av1.mkv > FATE_LAVF_CONTAINER_FATE-$(call ALLYES, H264_DEMUXER H264_PARSER MOV_MUXER) += h264.mp4 > FATE_LAVF_CONTAINER_FATE-$(call ALLYES, MATROSKA_DEMUXER OGG_MUXER) += vp3.ogg > FATE_LAVF_CONTAINER_FATE-$(call ALLYES, MATROSKA_DEMUXER OGV_MUXER) += vp8.ogg > @@ -86,8 +86,8 @@ FATE_LAVF_CONTAINER_FATE = $(FATE_LAVF_CONTAINER_FATE-yes:%=fate-lavf-fate-%) > $(FATE_LAVF_CONTAINER_FATE): REF = $(SRC_PATH)/tests/ref/lavf-fate/$(@:fate-lavf-fate-%=%) > $(FATE_LAVF_CONTAINER_FATE): $(AREF) $(VREF) > > -fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" > -fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" > +fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "-c:v av1" "-c:v copy" > +fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "-c:v av1" "-c:v copy" > fate-lavf-fate-evc.mp4: CMD = lavf_container_fate "evc/akiyo_cif.evc" "" "-c:v copy" > fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-c:v copy" > fate-lavf-fate-vp3.ogg: CMD = lavf_container_fate "vp3/coeff_level64.mkv" "-idct auto" > diff --git a/tests/ref/fate/av1-annexb-demux b/tests/ref/fate/av1-annexb-demux > index 77e0e378ab..139a893ec1 100644 > --- a/tests/ref/fate/av1-annexb-demux > +++ b/tests/ref/fate/av1-annexb-demux > @@ -3,7 +3,7 @@ > #media_type 0: video > #codec_id 0: av1 > #dimensions 0: 300x300 > -#sar 0: 1/1 > +#sar 0: 0/1 > 0, 0, 0, 48000, 12691, 0xf0adcc79 > 0, 48000, 48000, 48000, 4975, 0x1742a45f, F=0x0 > 0, 96000, 96000, 48000, 928, 0x7408be1a, F=0x0 > diff --git a/tests/ref/lavf-fate/av1.mkv b/tests/ref/lavf-fate/av1.mkv > index 84924fe05c..e6db3a814a 100644 > --- a/tests/ref/lavf-fate/av1.mkv > +++ b/tests/ref/lavf-fate/av1.mkv > @@ -1,3 +1,3 @@ > -ba198efa114b4db3c9f772728f84978b *tests/data/lavf-fate/lavf.av1.mkv > -55656 tests/data/lavf-fate/lavf.av1.mkv > +d875b42964dd55c8cea2e2752379d327 *tests/data/lavf-fate/lavf.av1.mkv > +55660 tests/data/lavf-fate/lavf.av1.mkv > tests/data/lavf-fate/lavf.av1.mkv CRC=0x7c27cc15 > diff --git a/tests/ref/lavf-fate/av1.mp4 b/tests/ref/lavf-fate/av1.mp4 > index ef174589ba..53b0cbe637 100644 > --- a/tests/ref/lavf-fate/av1.mp4 > +++ b/tests/ref/lavf-fate/av1.mp4 > @@ -1,3 +1,3 @@ > -d6afef355cf1bcf1a71b9cfb98c32fba *tests/data/lavf-fate/lavf.av1.mp4 > -55976 tests/data/lavf-fate/lavf.av1.mp4 > +194cd796f3d28a890b516b2507f57180 *tests/data/lavf-fate/lavf.av1.mp4 > +55960 tests/data/lavf-fate/lavf.av1.mp4 > tests/data/lavf-fate/lavf.av1.mp4 CRC=0x7c27cc15
On 9/7/2023 5:06 AM, Andreas Rheinhardt wrote: > James Almer: >> And remove the AVOID_PROBING flag, given it's the last av1 decoder to be tested >> either way. >> This fixes a regression introduced in 1652f2492f88434010053289d946dab6a57e4d58, >> where even if forcing the native av1 decoder, if another decoder was present, >> like libdav1d or libaom-av1, they'd be used for probing and some fate tests >> would have different results. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/av1dec.c | 8 ++++---- >> tests/fate/lavf-container.mak | 8 ++++---- >> tests/ref/fate/av1-annexb-demux | 2 +- >> tests/ref/lavf-fate/av1.mkv | 4 ++-- >> tests/ref/lavf-fate/av1.mp4 | 4 ++-- >> 5 files changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c >> index ec8401f4e0..a1e08185a7 100644 >> --- a/libavcodec/av1dec.c >> +++ b/libavcodec/av1dec.c >> @@ -612,6 +612,9 @@ static int get_pixel_format(AVCodecContext *avctx) >> if (ret < 0) >> return ret; >> >> + s->pix_fmt = pix_fmt; >> + avctx->pix_fmt = ret; >> + >> /** >> * check if the HW accel is inited correctly. If not, return un-implemented. >> * Since now the av1 decoder doesn't support native decode, if it will be >> @@ -623,9 +626,6 @@ static int get_pixel_format(AVCodecContext *avctx) >> return AVERROR(ENOSYS); > > Is the log message here actually accurate? The get_format callback > choosing the software pixel format does not mean that the hardware this > is run on or the lavc binary in use do not support hardware accelerated > AV1 decoding. It's not, but that's also unrelated to this patch. > (How is an API user actually supposed to know that decoding will fail if > the software pixel format is selected?) Other than the first returned ENOSYS, nothing. I could make it always return ENOSYS, same as before this patch, but anything we do will probably be hacky unless we add an actual software implementation.
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index ec8401f4e0..a1e08185a7 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -612,6 +612,9 @@ static int get_pixel_format(AVCodecContext *avctx) if (ret < 0) return ret; + s->pix_fmt = pix_fmt; + avctx->pix_fmt = ret; + /** * check if the HW accel is inited correctly. If not, return un-implemented. * Since now the av1 decoder doesn't support native decode, if it will be @@ -623,9 +626,6 @@ static int get_pixel_format(AVCodecContext *avctx) return AVERROR(ENOSYS); } - s->pix_fmt = pix_fmt; - avctx->pix_fmt = ret; - return 0; } @@ -1518,7 +1518,7 @@ const FFCodec ff_av1_decoder = { .init = av1_decode_init, .close = av1_decode_free, FF_CODEC_RECEIVE_FRAME_CB(av1_receive_frame), - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_AVOID_PROBING, + .p.capabilities = AV_CODEC_CAP_DR1, .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .flush = av1_decode_flush, .p.profiles = NULL_IF_CONFIG_SMALL(ff_av1_profiles), diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak index 0d4a224601..0081b45eea 100644 --- a/tests/fate/lavf-container.mak +++ b/tests/fate/lavf-container.mak @@ -70,9 +70,9 @@ fate-lavf-wtv: CMD = lavf_container "" "-c:a mp2 -threads 1" FATE_AVCONV += $(FATE_LAVF_CONTAINER) fate-lavf-container fate-lavf: $(FATE_LAVF_CONTAINER) -FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_PARSER MOV_MUXER) += av1.mp4 +FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_DECODER AV1_PARSER MOV_MUXER) += av1.mp4 +FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_DECODER AV1_PARSER MATROSKA_MUXER) += av1.mkv FATE_LAVF_CONTAINER_FATE-$(call ALLYES, EVC_DEMUXER EVC_PARSER MOV_MUXER) += evc.mp4 -FATE_LAVF_CONTAINER_FATE-$(call ALLYES, IVF_DEMUXER AV1_PARSER MATROSKA_MUXER) += av1.mkv FATE_LAVF_CONTAINER_FATE-$(call ALLYES, H264_DEMUXER H264_PARSER MOV_MUXER) += h264.mp4 FATE_LAVF_CONTAINER_FATE-$(call ALLYES, MATROSKA_DEMUXER OGG_MUXER) += vp3.ogg FATE_LAVF_CONTAINER_FATE-$(call ALLYES, MATROSKA_DEMUXER OGV_MUXER) += vp8.ogg @@ -86,8 +86,8 @@ FATE_LAVF_CONTAINER_FATE = $(FATE_LAVF_CONTAINER_FATE-yes:%=fate-lavf-fate-%) $(FATE_LAVF_CONTAINER_FATE): REF = $(SRC_PATH)/tests/ref/lavf-fate/$(@:fate-lavf-fate-%=%) $(FATE_LAVF_CONTAINER_FATE): $(AREF) $(VREF) -fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" -fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" +fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "-c:v av1" "-c:v copy" +fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "-c:v av1" "-c:v copy" fate-lavf-fate-evc.mp4: CMD = lavf_container_fate "evc/akiyo_cif.evc" "" "-c:v copy" fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-c:v copy" fate-lavf-fate-vp3.ogg: CMD = lavf_container_fate "vp3/coeff_level64.mkv" "-idct auto" diff --git a/tests/ref/fate/av1-annexb-demux b/tests/ref/fate/av1-annexb-demux index 77e0e378ab..139a893ec1 100644 --- a/tests/ref/fate/av1-annexb-demux +++ b/tests/ref/fate/av1-annexb-demux @@ -3,7 +3,7 @@ #media_type 0: video #codec_id 0: av1 #dimensions 0: 300x300 -#sar 0: 1/1 +#sar 0: 0/1 0, 0, 0, 48000, 12691, 0xf0adcc79 0, 48000, 48000, 48000, 4975, 0x1742a45f, F=0x0 0, 96000, 96000, 48000, 928, 0x7408be1a, F=0x0 diff --git a/tests/ref/lavf-fate/av1.mkv b/tests/ref/lavf-fate/av1.mkv index 84924fe05c..e6db3a814a 100644 --- a/tests/ref/lavf-fate/av1.mkv +++ b/tests/ref/lavf-fate/av1.mkv @@ -1,3 +1,3 @@ -ba198efa114b4db3c9f772728f84978b *tests/data/lavf-fate/lavf.av1.mkv -55656 tests/data/lavf-fate/lavf.av1.mkv +d875b42964dd55c8cea2e2752379d327 *tests/data/lavf-fate/lavf.av1.mkv +55660 tests/data/lavf-fate/lavf.av1.mkv tests/data/lavf-fate/lavf.av1.mkv CRC=0x7c27cc15 diff --git a/tests/ref/lavf-fate/av1.mp4 b/tests/ref/lavf-fate/av1.mp4 index ef174589ba..53b0cbe637 100644 --- a/tests/ref/lavf-fate/av1.mp4 +++ b/tests/ref/lavf-fate/av1.mp4 @@ -1,3 +1,3 @@ -d6afef355cf1bcf1a71b9cfb98c32fba *tests/data/lavf-fate/lavf.av1.mp4 -55976 tests/data/lavf-fate/lavf.av1.mp4 +194cd796f3d28a890b516b2507f57180 *tests/data/lavf-fate/lavf.av1.mp4 +55960 tests/data/lavf-fate/lavf.av1.mp4 tests/data/lavf-fate/lavf.av1.mp4 CRC=0x7c27cc15
And remove the AVOID_PROBING flag, given it's the last av1 decoder to be tested either way. This fixes a regression introduced in 1652f2492f88434010053289d946dab6a57e4d58, where even if forcing the native av1 decoder, if another decoder was present, like libdav1d or libaom-av1, they'd be used for probing and some fate tests would have different results. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/av1dec.c | 8 ++++---- tests/fate/lavf-container.mak | 8 ++++---- tests/ref/fate/av1-annexb-demux | 2 +- tests/ref/lavf-fate/av1.mkv | 4 ++-- tests/ref/lavf-fate/av1.mp4 | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-)