| Message ID | 20180128015045.32432-1-michael@niedermayer.cc |
|---|---|
| State | Superseded |
| Headers | show |
On 1/27/2018 10:50 PM, Michael Niedermayer wrote: > Regression since: c6939f65a116b1ffed345d29d8621ee4ffb32235 > Found-by: Paul B Mahol <onemda@gmail.com> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavfilter/vf_transpose.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c > index 1e1a5c4b89..bf2ab7eb18 100644 > --- a/libavfilter/vf_transpose.c > +++ b/libavfilter/vf_transpose.c > @@ -217,7 +217,7 @@ static int config_props_output(AVFilterLink *outlink) > > s->hsub = desc_in->log2_chroma_w; > s->vsub = desc_in->log2_chroma_h; > - s->planes = desc_in->nb_components; > + s->planes = (desc_in->flags & AV_PIX_FMT_FLAG_PLANAR) ? desc_in->nb_components : 1; > > av_assert0(desc_in->nb_components == desc_out->nb_components); If there are currently no tests for this filter (or at least not for packet formats), then one should be added. Lavfi's current fate coverage is unacceptably low. Something like two third of the codebase remains untested according to LCOV. At this point no software filter should be added without a fate test for at least a basic usage example using either the fate generated synthetic sources or any samples in the suite. Same with new features for new filters.
On 1/28/18, James Almer <jamrial@gmail.com> wrote: > On 1/27/2018 10:50 PM, Michael Niedermayer wrote: >> Regression since: c6939f65a116b1ffed345d29d8621ee4ffb32235 >> Found-by: Paul B Mahol <onemda@gmail.com> >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> libavfilter/vf_transpose.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c >> index 1e1a5c4b89..bf2ab7eb18 100644 >> --- a/libavfilter/vf_transpose.c >> +++ b/libavfilter/vf_transpose.c >> @@ -217,7 +217,7 @@ static int config_props_output(AVFilterLink *outlink) >> >> s->hsub = desc_in->log2_chroma_w; >> s->vsub = desc_in->log2_chroma_h; >> - s->planes = desc_in->nb_components; >> + s->planes = (desc_in->flags & AV_PIX_FMT_FLAG_PLANAR) ? >> desc_in->nb_components : 1; >> >> av_assert0(desc_in->nb_components == desc_out->nb_components); > > If there are currently no tests for this filter (or at least not for > packet formats), then one should be added. > > Lavfi's current fate coverage is unacceptably low. Something like two > third of the codebase remains untested according to LCOV. > At this point no software filter should be added without a fate test for > at least a basic usage example using either the fate generated synthetic > sources or any samples in the suite. Same with new features for new filters. That is unacceptable.
On 1/28/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > Regression since: c6939f65a116b1ffed345d29d8621ee4ffb32235 > Found-by: Paul B Mahol <onemda@gmail.com> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavfilter/vf_transpose.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c > index 1e1a5c4b89..bf2ab7eb18 100644 > --- a/libavfilter/vf_transpose.c > +++ b/libavfilter/vf_transpose.c > @@ -217,7 +217,7 @@ static int config_props_output(AVFilterLink *outlink) > > s->hsub = desc_in->log2_chroma_w; > s->vsub = desc_in->log2_chroma_h; > - s->planes = desc_in->nb_components; > + s->planes = (desc_in->flags & AV_PIX_FMT_FLAG_PLANAR) ? > desc_in->nb_components : 1; > > av_assert0(desc_in->nb_components == desc_out->nb_components); > > -- > 2.16.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > There is function that count number of planes.
On Sun, 28 Jan 2018 10:11:20 +0100 Paul B Mahol <onemda@gmail.com> wrote: > On 1/28/18, James Almer <jamrial@gmail.com> wrote: > > On 1/27/2018 10:50 PM, Michael Niedermayer wrote: > >> Regression since: c6939f65a116b1ffed345d29d8621ee4ffb32235 > >> Found-by: Paul B Mahol <onemda@gmail.com> > >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >> --- > >> libavfilter/vf_transpose.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c > >> index 1e1a5c4b89..bf2ab7eb18 100644 > >> --- a/libavfilter/vf_transpose.c > >> +++ b/libavfilter/vf_transpose.c > >> @@ -217,7 +217,7 @@ static int config_props_output(AVFilterLink *outlink) > >> > >> s->hsub = desc_in->log2_chroma_w; > >> s->vsub = desc_in->log2_chroma_h; > >> - s->planes = desc_in->nb_components; > >> + s->planes = (desc_in->flags & AV_PIX_FMT_FLAG_PLANAR) ? > >> desc_in->nb_components : 1; > >> > >> av_assert0(desc_in->nb_components == desc_out->nb_components); > > > > If there are currently no tests for this filter (or at least not for > > packet formats), then one should be added. > > > > Lavfi's current fate coverage is unacceptably low. Something like two > > third of the codebase remains untested according to LCOV. > > At this point no software filter should be added without a fate test for > > at least a basic usage example using either the fate generated synthetic > > sources or any samples in the suite. Same with new features for new filters. > > That is unacceptable. Testing code is unacceptable? Excuse me? The only unacceptable thing is probably the slowdown you can expect on full fate runs once the coverage reaches 100%.
James Almer (2018-01-27): > If there are currently no tests for this filter (or at least not for > packet formats), then one should be added. > > Lavfi's current fate coverage is unacceptably low. Something like two > third of the codebase remains untested according to LCOV. > At this point no software filter should be added without a fate test for > at least a basic usage example using either the fate generated synthetic > sources or any samples in the suite. Same with new features for new filters. I think it is a reasonable requirement. Regards,
On Sun, Jan 28, 2018 at 10:12:19AM +0100, Paul B Mahol wrote: > On 1/28/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > > Regression since: c6939f65a116b1ffed345d29d8621ee4ffb32235 > > Found-by: Paul B Mahol <onemda@gmail.com> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavfilter/vf_transpose.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c > > index 1e1a5c4b89..bf2ab7eb18 100644 > > --- a/libavfilter/vf_transpose.c > > +++ b/libavfilter/vf_transpose.c > > @@ -217,7 +217,7 @@ static int config_props_output(AVFilterLink *outlink) > > > > s->hsub = desc_in->log2_chroma_w; > > s->vsub = desc_in->log2_chroma_h; > > - s->planes = desc_in->nb_components; > > + s->planes = (desc_in->flags & AV_PIX_FMT_FLAG_PLANAR) ? > > desc_in->nb_components : 1; > > > > av_assert0(desc_in->nb_components == desc_out->nb_components); > > > > -- > > 2.16.1 > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > There is function that count number of planes. yes, just realized that too, i will change the code to use that thx [...]
On Sat, Jan 27, 2018 at 11:24:36PM -0300, James Almer wrote: > On 1/27/2018 10:50 PM, Michael Niedermayer wrote: > > Regression since: c6939f65a116b1ffed345d29d8621ee4ffb32235 > > Found-by: Paul B Mahol <onemda@gmail.com> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavfilter/vf_transpose.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c > > index 1e1a5c4b89..bf2ab7eb18 100644 > > --- a/libavfilter/vf_transpose.c > > +++ b/libavfilter/vf_transpose.c > > @@ -217,7 +217,7 @@ static int config_props_output(AVFilterLink *outlink) > > > > s->hsub = desc_in->log2_chroma_w; > > s->vsub = desc_in->log2_chroma_h; > > - s->planes = desc_in->nb_components; > > + s->planes = (desc_in->flags & AV_PIX_FMT_FLAG_PLANAR) ? desc_in->nb_components : 1; > > > > av_assert0(desc_in->nb_components == desc_out->nb_components); > > If there are currently no tests for this filter (or at least not for > packet formats), then one should be added. patch posted that tests transpose more completely pixel format wise thanks [...]
diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c index 1e1a5c4b89..bf2ab7eb18 100644 --- a/libavfilter/vf_transpose.c +++ b/libavfilter/vf_transpose.c @@ -217,7 +217,7 @@ static int config_props_output(AVFilterLink *outlink) s->hsub = desc_in->log2_chroma_w; s->vsub = desc_in->log2_chroma_h; - s->planes = desc_in->nb_components; + s->planes = (desc_in->flags & AV_PIX_FMT_FLAG_PLANAR) ? desc_in->nb_components : 1; av_assert0(desc_in->nb_components == desc_out->nb_components);
Regression since: c6939f65a116b1ffed345d29d8621ee4ffb32235 Found-by: Paul B Mahol <onemda@gmail.com> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavfilter/vf_transpose.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)