Re: [FFmpeg-devel] [PATCH] Select cubic and lanczos as alternative where super-sampling is not supported

[Moritz Barsnick]

On Sun, Sep 11, 2016 at 19:04:39 +0100, Sven C. Dack wrote:
> the second body if it entered the first, because the value is part of both 
> the 
> conditions.

D'uh, I missed that part. (It wasn't obvious enough to me. ;-))

> I thought about some neat logic at first, but decided to do it this
> way for more readability. Now you've put me on the spot... Your call.

In that case, the code you wrote is just fine, albeit a bit more
difficult to grasp - for me.

It's not my call at all, I'm just commenting.

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Select cubic and lanczos as alternative where super-sampling is not supported

[Sven C. Dack]

On 11/09/16 18:55, Moritz Barsnick wrote:

Moin Sven,

On Fri, Sep 09, 2016 at 11:13:55 +0100, Sven C. Dack wrote:

I may be missing something, but my excuse is that I can't test, but just
inspect by looking at it:


+   if (s->interp_algo == NPPI_INTER_SUPER &&
+   (out_width > in_width && out_height > in_height)) {
+   s->interp_algo = NPPI_INTER_LANCZOS;
+   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for output 
dimensions, using lanczos instead.\n");
+   }
+   if (s->interp_algo == NPPI_INTER_SUPER &&
+   !(out_width < in_width && out_height < in_height)) {
+   s->interp_algo = NPPI_INTER_CUBIC;
+   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for output 
dimensions, using cubic instead.\n");
+   }
+}


The value s->interp_algo gets change in each body. There is no way it can enter 
the second body if it entered the first, because the value is part of both the 
conditions.


I thought about some neat logic at first, but decided to do it this way for more 
readability. Now you've put me on the spot... Your call.


Sven

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Select cubic and lanczos as alternative where super-sampling is not supported

[Moritz Barsnick]

Moin Sven,

On Fri, Sep 09, 2016 at 11:13:55 +0100, Sven C. Dack wrote:

I may be missing something, but my excuse is that I can't test, but just
inspect by looking at it:

> +   if (s->interp_algo == NPPI_INTER_SUPER &&
> +   (out_width > in_width && out_height > in_height)) {
> +   s->interp_algo = NPPI_INTER_LANCZOS;
> +   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for 
> output dimensions, using lanczos instead.\n");
> +   }
> +   if (s->interp_algo == NPPI_INTER_SUPER &&
> +   !(out_width < in_width && out_height < in_height)) {
> +   s->interp_algo = NPPI_INTER_CUBIC;
> +   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for 
> output dimensions, using cubic instead.\n");
> +   }
> +}

Let's assume s->interp_algo=NPPI_INTER_SUPER, out_width>in_width,
out_height>in_height:

if (true && (true && true)) {  }
if (true && !(false && false) {  }

Both blocks will be entered! Didn't you see both messages when testing?

Your commit message says ffmpeg needs to choose a different algo if
both dimensions aren't smaller, that's the second if(), which should be
the top-level decider:

> +   if (s->interp_algo == NPPI_INTER_SUPER &&
> +   !(out_width < in_width && out_height < in_height)) {

Then you seem to choose lanczos only if both are greater, so let me do
this as such:
> +   if (out_width > in_width && out_height > in_height) {
> +   s->interp_algo = NPPI_INTER_LANCZOS;
> +   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for 
> output dimensions, using lanczos instead.\n");
> +   }
then the rest (i.e. one larger or equal, one smaller) would be cubic:
> +   else {
> +   s->interp_algo = NPPI_INTER_CUBIC;
> +   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for 
> output dimensions, using cubic instead.\n");
> +   }
> +   }

Was that the intent?

Gruß,
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Select cubic and lanczos as alternative where super-sampling is not supported

[Carl Eugen Hoyos]

2016-09-09 12:13 GMT+02:00 Sven C. Dack :

> It tells me my change log entry is missing.
>
> I've commited it with: git commit vf_scale_npp.c
>
> Entered my comment and applied format-patch: git format-patch -1
> vf_scale_npp.c
>
> How would I get the missing change log entry in there?

The change to the file Changelog in FFmpeg's root is only
necessary if you make a major addition to FFmpeg (like a
new filter, not a fix for an existing filter).

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Select cubic and lanczos as alternative where super-sampling is not supported

[Sven C. Dack]

On 09/09/16 11:03, Carl Eugen Hoyos wrote:

2016-09-09 11:55 GMT+02:00 Sven C. Dack :


Super-sampling is currently only supported by CUDA/NPP when the output
dimensions are both smaller than the input dimensions. The patch lets ffmpeg
select an alternative algorithm and prints a warning in such cases.

Tabs (as in your patch) cannot be committed to our git repository, please
remove them.
You can use the script tools/patcheck to find them.



Ok, thank you.

It tells me my change log entry is missing.

I've commited it with: git commit vf_scale_npp.c

Entered my comment and applied format-patch: git format-patch -1 vf_scale_npp.c

How would I get the missing change log entry in there?

Sven

>From aacd8ecc2f39a45bbfdf6780d9b3b13e6ed0fb41 Mon Sep 17 00:00:00 2001
From: "Sven C. Dack" 
Date: Fri, 9 Sep 2016 10:18:07 +0100
Subject: [PATCH] Select cubic and lanczos as alternative where super-sampling
 is not supported

---
 libavfilter/vf_scale_npp.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_scale_npp.c b/libavfilter/vf_scale_npp.c
index 68cee39..82ba2f4 100644
--- a/libavfilter/vf_scale_npp.c
+++ b/libavfilter/vf_scale_npp.c
@@ -294,10 +294,21 @@ static int init_processing_chain(AVFilterContext *ctx, int in_width, int in_heig
 
 /* figure out which stages need to be done */
 if (in_width != out_width || in_height != out_height ||
-in_deinterleaved_format != out_deinterleaved_format)
+in_deinterleaved_format != out_deinterleaved_format) {
 s->stages[STAGE_RESIZE].stage_needed = 1;
 
+   if (s->interp_algo == NPPI_INTER_SUPER &&
+   (out_width > in_width && out_height > in_height)) {
+   s->interp_algo = NPPI_INTER_LANCZOS;
+   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for output dimensions, using lanczos instead.\n");
+   }
+   if (s->interp_algo == NPPI_INTER_SUPER &&
+   !(out_width < in_width && out_height < in_height)) {
+   s->interp_algo = NPPI_INTER_CUBIC;
+   av_log(ctx, AV_LOG_WARNING, "super-sampling not supported for output dimensions, using cubic instead.\n");
+   }
+}
+
 if (!s->stages[STAGE_RESIZE].stage_needed && in_format == out_format)
 s->passthrough = 1;
 
-- 
2.9.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Select cubic and lanczos as alternative where super-sampling is not supported

[Carl Eugen Hoyos]

2016-09-09 11:55 GMT+02:00 Sven C. Dack :

> Super-sampling is currently only supported by CUDA/NPP when the output
> dimensions are both smaller than the input dimensions. The patch lets ffmpeg
> select an alternative algorithm and prints a warning in such cases.

Tabs (as in your patch) cannot be committed to our git repository, please
remove them.
You can use the script tools/patcheck to find them.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel