| Message ID | GV1P250MB0737239BB086232B19900BAA8F529@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avutil/pixdesc: Remove always-false checks | 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 |
Quoting Andreas Rheinhardt (2022-09-26 21:58:55) > Instead use av_pix_fmt_desc_next(). It is still possible > to check its return values by comparing it with the > (currently) expected values and the code does so. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavutil/pixdesc.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c > index c42a0242c5..f6755f41df 100644 > --- a/libavutil/pixdesc.c > +++ b/libavutil/pixdesc.c > @@ -2913,10 +2913,10 @@ int av_pix_fmt_count_planes(enum AVPixelFormat pix_fmt) > } > > void ff_check_pixfmt_descriptors(void){ > - int i, j; > + const AVPixFmtDescriptor *d, *last = NULL; > + int i; > > - for (i=0; i<FF_ARRAY_ELEMS(av_pix_fmt_descriptors); i++) { > - const AVPixFmtDescriptor *d = &av_pix_fmt_descriptors[i]; > + for (i = AV_PIX_FMT_NONE, d = NULL; i++, d = av_pix_fmt_desc_next(d);) { > uint8_t fill[4][8+6+3] = {{0}}; > uint8_t *data[4] = {fill[0], fill[1], fill[2], fill[3]}; > int linesize[4] = {0,0,0,0}; > @@ -2927,9 +2927,15 @@ void ff_check_pixfmt_descriptors(void){ > av_assert0(d->log2_chroma_w <= 3); > av_assert0(d->log2_chroma_h <= 3); > av_assert0(d->nb_components <= 4); > - av_assert2(av_get_pix_fmt(d->name) == i); > + av_assert2(av_get_pix_fmt(d->name) == av_pix_fmt_desc_get_id(d)); > + > + /* The following two checks as well as the one after the loop > + * would need to be changed if we changed the way the descriptors > + * are stored. */ > + av_assert0(i == av_pix_fmt_desc_get_id(d)); > + av_assert0(!last || last + 1 == d); Don't see last being set.
Anton Khirnov: > Quoting Andreas Rheinhardt (2022-09-26 21:58:55) >> Instead use av_pix_fmt_desc_next(). It is still possible >> to check its return values by comparing it with the >> (currently) expected values and the code does so. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavutil/pixdesc.c | 17 ++++++++++++----- >> 1 file changed, 12 insertions(+), 5 deletions(-) >> >> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c >> index c42a0242c5..f6755f41df 100644 >> --- a/libavutil/pixdesc.c >> +++ b/libavutil/pixdesc.c >> @@ -2913,10 +2913,10 @@ int av_pix_fmt_count_planes(enum AVPixelFormat pix_fmt) >> } >> >> void ff_check_pixfmt_descriptors(void){ >> - int i, j; >> + const AVPixFmtDescriptor *d, *last = NULL; >> + int i; >> >> - for (i=0; i<FF_ARRAY_ELEMS(av_pix_fmt_descriptors); i++) { >> - const AVPixFmtDescriptor *d = &av_pix_fmt_descriptors[i]; >> + for (i = AV_PIX_FMT_NONE, d = NULL; i++, d = av_pix_fmt_desc_next(d);) { >> uint8_t fill[4][8+6+3] = {{0}}; >> uint8_t *data[4] = {fill[0], fill[1], fill[2], fill[3]}; >> int linesize[4] = {0,0,0,0}; >> @@ -2927,9 +2927,15 @@ void ff_check_pixfmt_descriptors(void){ >> av_assert0(d->log2_chroma_w <= 3); >> av_assert0(d->log2_chroma_h <= 3); >> av_assert0(d->nb_components <= 4); >> - av_assert2(av_get_pix_fmt(d->name) == i); >> + av_assert2(av_get_pix_fmt(d->name) == av_pix_fmt_desc_get_id(d)); >> + >> + /* The following two checks as well as the one after the loop >> + * would need to be changed if we changed the way the descriptors >> + * are stored. */ >> + av_assert0(i == av_pix_fmt_desc_get_id(d)); >> + av_assert0(!last || last + 1 == d); > > Don't see last being set. > Correct. Will fix. - Andreas
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c index c42a0242c5..f6755f41df 100644 --- a/libavutil/pixdesc.c +++ b/libavutil/pixdesc.c @@ -2913,10 +2913,10 @@ int av_pix_fmt_count_planes(enum AVPixelFormat pix_fmt) } void ff_check_pixfmt_descriptors(void){ - int i, j; + const AVPixFmtDescriptor *d, *last = NULL; + int i; - for (i=0; i<FF_ARRAY_ELEMS(av_pix_fmt_descriptors); i++) { - const AVPixFmtDescriptor *d = &av_pix_fmt_descriptors[i]; + for (i = AV_PIX_FMT_NONE, d = NULL; i++, d = av_pix_fmt_desc_next(d);) { uint8_t fill[4][8+6+3] = {{0}}; uint8_t *data[4] = {fill[0], fill[1], fill[2], fill[3]}; int linesize[4] = {0,0,0,0}; @@ -2927,9 +2927,15 @@ void ff_check_pixfmt_descriptors(void){ av_assert0(d->log2_chroma_w <= 3); av_assert0(d->log2_chroma_h <= 3); av_assert0(d->nb_components <= 4); - av_assert2(av_get_pix_fmt(d->name) == i); + av_assert2(av_get_pix_fmt(d->name) == av_pix_fmt_desc_get_id(d)); + + /* The following two checks as well as the one after the loop + * would need to be changed if we changed the way the descriptors + * are stored. */ + av_assert0(i == av_pix_fmt_desc_get_id(d)); + av_assert0(!last || last + 1 == d); - for (j=0; j<FF_ARRAY_ELEMS(d->comp); j++) { + for (int j = 0; j < FF_ARRAY_ELEMS(d->comp); j++) { const AVComponentDescriptor *c = &d->comp[j]; if(j>=d->nb_components) { av_assert0(!c->plane && !c->step && !c->offset && !c->shift && !c->depth); @@ -2948,6 +2954,7 @@ void ff_check_pixfmt_descriptors(void){ av_write_image_line(tmp, data, linesize, d, 0, 0, j, 2); } } + av_assert0(i == AV_PIX_FMT_NB); }
Instead use av_pix_fmt_desc_next(). It is still possible to check its return values by comparing it with the (currently) expected values and the code does so. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavutil/pixdesc.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)