| Message ID | 1597159169-2380-1-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/vf_w3fdif: 255 * 256 * 128 -> max | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
NAK On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > max is initialized to ((1 << depth) - 1) * 256 * 128 before. > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavfilter/vf_w3fdif.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_w3fdif.c b/libavfilter/vf_w3fdif.c > index 5d64dbd..0f9efbf 100644 > --- a/libavfilter/vf_w3fdif.c > +++ b/libavfilter/vf_w3fdif.c > @@ -163,7 +163,7 @@ static void filter_scale(uint8_t *out_pixel, const > int32_t *work_pixel, int line > int j; > > for (j = 0; j < linesize; j++, out_pixel++, work_pixel++) > - *out_pixel = av_clip(*work_pixel, 0, 255 * 256 * 128) >> 15; > + *out_pixel = av_clip(*work_pixel, 0, max) >> 15; > } > > static void filter16_simple_low(int32_t *work_line, > -- > 1.8.3.1 > > _______________________________________________ > 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". Change is pointless and give no real benefits.
On Tue, Aug 11, 2020 at 05:24:06PM +0200, Paul B Mahol wrote: > NAK > > On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: > > From: Limin Wang <lance.lmwang@gmail.com> > > > > max is initialized to ((1 << depth) - 1) * 256 * 128 before. > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavfilter/vf_w3fdif.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavfilter/vf_w3fdif.c b/libavfilter/vf_w3fdif.c > > index 5d64dbd..0f9efbf 100644 > > --- a/libavfilter/vf_w3fdif.c > > +++ b/libavfilter/vf_w3fdif.c > > @@ -163,7 +163,7 @@ static void filter_scale(uint8_t *out_pixel, const > > int32_t *work_pixel, int line > > int j; > > > > for (j = 0; j < linesize; j++, out_pixel++, work_pixel++) > > - *out_pixel = av_clip(*work_pixel, 0, 255 * 256 * 128) >> 15; > > + *out_pixel = av_clip(*work_pixel, 0, max) >> 15; > > } > > > > static void filter16_simple_low(int32_t *work_line, > > -- > > 1.8.3.1 > > > > _______________________________________________ > > 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". > > > Change is pointless and give no real benefits. 1. max is input as parameter and calculated already, why use it is pointless? 2. keep 8 bit and 16bit with same code so that we can remove the duplicate code later.
On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: > On Tue, Aug 11, 2020 at 05:24:06PM +0200, Paul B Mahol wrote: >> NAK >> >> On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: >> > From: Limin Wang <lance.lmwang@gmail.com> >> > >> > max is initialized to ((1 << depth) - 1) * 256 * 128 before. >> > >> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >> > --- >> > libavfilter/vf_w3fdif.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/libavfilter/vf_w3fdif.c b/libavfilter/vf_w3fdif.c >> > index 5d64dbd..0f9efbf 100644 >> > --- a/libavfilter/vf_w3fdif.c >> > +++ b/libavfilter/vf_w3fdif.c >> > @@ -163,7 +163,7 @@ static void filter_scale(uint8_t *out_pixel, const >> > int32_t *work_pixel, int line >> > int j; >> > >> > for (j = 0; j < linesize; j++, out_pixel++, work_pixel++) >> > - *out_pixel = av_clip(*work_pixel, 0, 255 * 256 * 128) >> 15; >> > + *out_pixel = av_clip(*work_pixel, 0, max) >> 15; >> > } >> > >> > static void filter16_simple_low(int32_t *work_line, >> > -- >> > 1.8.3.1 >> > >> > _______________________________________________ >> > 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". >> >> >> Change is pointless and give no real benefits. > > 1. max is input as parameter and calculated already, why use it is > pointless? > 2. keep 8 bit and 16bit with same code so that we can remove the duplicate > code > later. Code is not duplicated at all. Different types are used. You can off course use macros if you are really bored but regressions in performance are unacceptable. Your patch also ruin performance. > > -- > Thanks, > Limin Wang > _______________________________________________ > 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, Aug 11, 2020 at 05:41:58PM +0200, Paul B Mahol wrote: > On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: > > On Tue, Aug 11, 2020 at 05:24:06PM +0200, Paul B Mahol wrote: > >> NAK > >> > >> On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: > >> > From: Limin Wang <lance.lmwang@gmail.com> > >> > > >> > max is initialized to ((1 << depth) - 1) * 256 * 128 before. > >> > > >> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >> > --- > >> > libavfilter/vf_w3fdif.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/libavfilter/vf_w3fdif.c b/libavfilter/vf_w3fdif.c > >> > index 5d64dbd..0f9efbf 100644 > >> > --- a/libavfilter/vf_w3fdif.c > >> > +++ b/libavfilter/vf_w3fdif.c > >> > @@ -163,7 +163,7 @@ static void filter_scale(uint8_t *out_pixel, const > >> > int32_t *work_pixel, int line > >> > int j; > >> > > >> > for (j = 0; j < linesize; j++, out_pixel++, work_pixel++) > >> > - *out_pixel = av_clip(*work_pixel, 0, 255 * 256 * 128) >> 15; > >> > + *out_pixel = av_clip(*work_pixel, 0, max) >> 15; > >> > } > >> > > >> > static void filter16_simple_low(int32_t *work_line, > >> > -- > >> > 1.8.3.1 > >> > > >> > _______________________________________________ > >> > 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". > >> > >> > >> Change is pointless and give no real benefits. > > > > 1. max is input as parameter and calculated already, why use it is > > pointless? > > 2. keep 8 bit and 16bit with same code so that we can remove the duplicate > > code > > later. > > Code is not duplicated at all. Different types are used. > You can off course use macros if you are really bored but regressions > in performance are unacceptable. > Your patch also ruin performance. I haven't do performance testing for the filter have 8bit x86 asm code, please ignore it if it'll impact the performance. > > > > > -- > > Thanks, > > Limin Wang > > _______________________________________________ > > 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".
On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: > On Tue, Aug 11, 2020 at 05:41:58PM +0200, Paul B Mahol wrote: >> On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: >> > On Tue, Aug 11, 2020 at 05:24:06PM +0200, Paul B Mahol wrote: >> >> NAK >> >> >> >> On 8/11/20, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote: >> >> > From: Limin Wang <lance.lmwang@gmail.com> >> >> > >> >> > max is initialized to ((1 << depth) - 1) * 256 * 128 before. >> >> > >> >> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >> >> > --- >> >> > libavfilter/vf_w3fdif.c | 2 +- >> >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> > >> >> > diff --git a/libavfilter/vf_w3fdif.c b/libavfilter/vf_w3fdif.c >> >> > index 5d64dbd..0f9efbf 100644 >> >> > --- a/libavfilter/vf_w3fdif.c >> >> > +++ b/libavfilter/vf_w3fdif.c >> >> > @@ -163,7 +163,7 @@ static void filter_scale(uint8_t *out_pixel, >> >> > const >> >> > int32_t *work_pixel, int line >> >> > int j; >> >> > >> >> > for (j = 0; j < linesize; j++, out_pixel++, work_pixel++) >> >> > - *out_pixel = av_clip(*work_pixel, 0, 255 * 256 * 128) >> 15; >> >> > + *out_pixel = av_clip(*work_pixel, 0, max) >> 15; >> >> > } >> >> > >> >> > static void filter16_simple_low(int32_t *work_line, >> >> > -- >> >> > 1.8.3.1 >> >> > >> >> > _______________________________________________ >> >> > 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". >> >> >> >> >> >> Change is pointless and give no real benefits. >> > >> > 1. max is input as parameter and calculated already, why use it is >> > pointless? >> > 2. keep 8 bit and 16bit with same code so that we can remove the >> > duplicate >> > code >> > later. >> >> Code is not duplicated at all. Different types are used. >> You can off course use macros if you are really bored but regressions >> in performance are unacceptable. >> Your patch also ruin performance. > > I haven't do performance testing for the filter have 8bit x86 asm code, > please > ignore it if it'll impact the performance. Ah, yes, just write macro for this two scale functions if you care and makes sense. > >> >> > >> > -- >> > Thanks, >> > Limin Wang >> > _______________________________________________ >> > 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". > > -- > Thanks, > Limin Wang > _______________________________________________ > 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/libavfilter/vf_w3fdif.c b/libavfilter/vf_w3fdif.c index 5d64dbd..0f9efbf 100644 --- a/libavfilter/vf_w3fdif.c +++ b/libavfilter/vf_w3fdif.c @@ -163,7 +163,7 @@ static void filter_scale(uint8_t *out_pixel, const int32_t *work_pixel, int line int j; for (j = 0; j < linesize; j++, out_pixel++, work_pixel++) - *out_pixel = av_clip(*work_pixel, 0, 255 * 256 * 128) >> 15; + *out_pixel = av_clip(*work_pixel, 0, max) >> 15; } static void filter16_simple_low(int32_t *work_line,