Re: [FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-08-23 Thread Paul B Mahol
On 6/12/20, Gavin Smith  wrote:
>
> On 12/06/2020 16:58, Gavin Smith wrote:
>> On 12/06/2020 12:59, Timo Rothenpieler wrote:
>>> On 12.06.2020 00:31, Gavin Smith wrote:

 On 11/06/2020 22:03, Timo Rothenpieler wrote:
> On 11.06.2020 22:27, Gavin Smith wrote:
>> This is to address trac ticket #8724. The filter did not preserve
>> alpha transparency. Items that were transparent in the input would
>> appear black on the output or pixels that were semi-tranparent
>> would appear opaque.
>>
>
> What is the performance impact for inputs without an alpha channel?

 Firstly, I'm new to this world of filters.  Now my patch only
 applies to chromakey and not chromahold. On that note, is it not the
 case that the chromakey mandates that all its inputs surfaces
 contain alpha (as per query_formats function)? Would any
 AVPixelFormat that does not match query_formats get converted to a
 format containing alpha transparency?
>>>
>>> But it still adds new code, and given it's in the very hot path of
>>> the filter, it can easily add a performance penalty.
>> Sure. I understand. I'll see what numbers I can get.  Having a quick
>> cursory look over the code, I wonder if my code actually might be
>> slightly more performant because it checks the case if a0 != 0.
>> However, this jmp/cmp may negatively impact performance.
>
> vf_chromakey wll now have 2 implementations of chromakey depending on
> whether "use_alpha" is selected. What are your feelings on the best way
> to approach this?
>
> 1. Add an additional function pointer to ChromakeyContext: In
> do_chromakey_slice(..), call the said func ptr in the loop each iteration?

This is best solution.
You could also add function pointer for do_chromakey_pixel(), which
would pass in 2nd argument current alpha value.
In one case that 2nd argument would not be used and in different case
it will be used.

>
> 2. Add a conditional to do_chromakey_slice(...): In the loop iteration,
> check "use_alpha" variable and execute and jmp to pertinent code?

That would be slow.

>
> 3. Any other suggestions?
>
> Thanks.
>
>>>
>
> Generally looks fine to me, but might need hidden behind an option,
> as to not break existing setups that rely on this filter
> discarding/ignoring the input alpha channel.

 Yes. No problem. "preserve_transparency" and default to the
 equivalent of 'false'?
>>>
>>> That seems a bit clunky. Maybe something like "use_alpha"?
>> Sure.
>>>
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-requ...@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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-06-12 Thread Gavin Smith


On 12/06/2020 16:58, Gavin Smith wrote:

On 12/06/2020 12:59, Timo Rothenpieler wrote:

On 12.06.2020 00:31, Gavin Smith wrote:


On 11/06/2020 22:03, Timo Rothenpieler wrote:

On 11.06.2020 22:27, Gavin Smith wrote:
This is to address trac ticket #8724. The filter did not preserve 
alpha transparency. Items that were transparent in the input would 
appear black on the output or pixels that were semi-tranparent 
would appear opaque.




What is the performance impact for inputs without an alpha channel?


Firstly, I'm new to this world of filters.  Now my patch only 
applies to chromakey and not chromahold. On that note, is it not the 
case that the chromakey mandates that all its inputs surfaces 
contain alpha (as per query_formats function)? Would any 
AVPixelFormat that does not match query_formats get converted to a 
format containing alpha transparency?


But it still adds new code, and given it's in the very hot path of 
the filter, it can easily add a performance penalty.
Sure. I understand. I'll see what numbers I can get.  Having a quick 
cursory look over the code, I wonder if my code actually might be 
slightly more performant because it checks the case if a0 != 0. 
However, this jmp/cmp may negatively impact performance.


vf_chromakey wll now have 2 implementations of chromakey depending on 
whether "use_alpha" is selected. What are your feelings on the best way 
to approach this?


1. Add an additional function pointer to ChromakeyContext: In 
do_chromakey_slice(..), call the said func ptr in the loop each iteration?


2. Add a conditional to do_chromakey_slice(...): In the loop iteration, 
check "use_alpha" variable and execute and jmp to pertinent code?


3. Any other suggestions?

Thanks.





Generally looks fine to me, but might need hidden behind an option, 
as to not break existing setups that rely on this filter 
discarding/ignoring the input alpha channel.


Yes. No problem. "preserve_transparency" and default to the 
equivalent of 'false'?


That seems a bit clunky. Maybe something like "use_alpha"?

Sure.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-06-12 Thread Gavin Smith

On 12/06/2020 12:59, Timo Rothenpieler wrote:

On 12.06.2020 00:31, Gavin Smith wrote:


On 11/06/2020 22:03, Timo Rothenpieler wrote:

On 11.06.2020 22:27, Gavin Smith wrote:
This is to address trac ticket #8724. The filter did not preserve 
alpha transparency. Items that were transparent in the input would 
appear black on the output or pixels that were semi-tranparent 
would appear opaque.




What is the performance impact for inputs without an alpha channel?


Firstly, I'm new to this world of filters.  Now my patch only applies 
to chromakey and not chromahold. On that note, is it not the case 
that the chromakey mandates that all its inputs surfaces contain 
alpha (as per query_formats function)? Would any AVPixelFormat that 
does not match query_formats get converted to a format containing 
alpha transparency?


But it still adds new code, and given it's in the very hot path of the 
filter, it can easily add a performance penalty.
Sure. I understand. I'll see what numbers I can get.  Having a quick 
cursory look over the code, I wonder if my code actually might be 
slightly more performant because it checks the case if a0 != 0. However, 
this jmp/cmp may negatively impact performance.




Generally looks fine to me, but might need hidden behind an option, 
as to not break existing setups that rely on this filter 
discarding/ignoring the input alpha channel.


Yes. No problem. "preserve_transparency" and default to the 
equivalent of 'false'?


That seems a bit clunky. Maybe something like "use_alpha"?

Sure.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-06-12 Thread Timo Rothenpieler

On 12.06.2020 00:31, Gavin Smith wrote:


On 11/06/2020 22:03, Timo Rothenpieler wrote:

On 11.06.2020 22:27, Gavin Smith wrote:
This is to address trac ticket #8724.  The filter did not preserve 
alpha transparency. Items that were transparent in the input would 
appear black on the output or pixels that were semi-tranparent would 
appear opaque.




What is the performance impact for inputs without an alpha channel?


Firstly, I'm new to this world of filters.  Now my patch only applies to 
chromakey and not chromahold. On that note, is it not the case that the 
chromakey mandates that all its inputs surfaces contain alpha (as per 
query_formats function)? Would any AVPixelFormat that does not match 
query_formats get converted to a format containing alpha transparency?


But it still adds new code, and given it's in the very hot path of the 
filter, it can easily add a performance penalty.




Generally looks fine to me, but might need hidden behind an option, as 
to not break existing setups that rely on this filter 
discarding/ignoring the input alpha channel.


Yes. No problem. "preserve_transparency" and default to the equivalent 
of 'false'?


That seems a bit clunky. Maybe something like "use_alpha"?

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-06-11 Thread Gavin Smith


On 11/06/2020 22:03, Timo Rothenpieler wrote:

On 11.06.2020 22:27, Gavin Smith wrote:
This is to address trac ticket #8724.  The filter did not preserve 
alpha transparency. Items that were transparent in the input would 
appear black on the output or pixels that were semi-tranparent would 
appear opaque.




What is the performance impact for inputs without an alpha channel?


Firstly, I'm new to this world of filters.  Now my patch only applies to 
chromakey and not chromahold. On that note, is it not the case that the 
chromakey mandates that all its inputs surfaces contain alpha (as per 
query_formats function)? Would any AVPixelFormat that does not match 
query_formats get converted to a format containing alpha transparency?




Generally looks fine to me, but might need hidden behind an option, as 
to not break existing setups that rely on this filter 
discarding/ignoring the input alpha channel.


Yes. No problem. "preserve_transparency" and default to the equivalent 
of 'false'?



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-06-11 Thread Timo Rothenpieler

On 11.06.2020 22:27, Gavin Smith wrote:

This is to address trac ticket #8724.  The filter did not preserve alpha 
transparency. Items that were transparent in the input would appear black on 
the output or pixels that were semi-tranparent would appear opaque.



What is the performance impact for inputs without an alpha channel?

Generally looks fine to me, but might need hidden behind an option, as 
to not break existing setups that rely on this filter 
discarding/ignoring the input alpha channel.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-06-11 Thread Gavin Smith
This is to address trac ticket #8724.  The filter did not preserve alpha 
transparency. Items that were transparent in the input would appear black on 
the output or pixels that were semi-tranparent would appear opaque.
---
 libavfilter/vf_chromakey.c | 43 ++
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/libavfilter/vf_chromakey.c b/libavfilter/vf_chromakey.c
index 4b1669d084..7206f1 100644
--- a/libavfilter/vf_chromakey.c
+++ b/libavfilter/vf_chromakey.c
@@ -47,7 +47,7 @@ typedef struct ChromakeyContext {
 int jobnr, int nb_jobs);
 } ChromakeyContext;
 
-static uint8_t do_chromakey_pixel(ChromakeyContext *ctx, uint8_t u[9], uint8_t 
v[9])
+static uint8_t do_chromakey_pixel(ChromakeyContext *ctx, uint8_t a, uint8_t 
u[9], uint8_t v[9])
 {
 double diff = 0.0;
 int du, dv, i;
@@ -62,13 +62,13 @@ static uint8_t do_chromakey_pixel(ChromakeyContext *ctx, 
uint8_t u[9], uint8_t v
 diff /= 9.0;
 
 if (ctx->blend > 0.0001) {
-return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * 
255.0;
+return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * 
(float)a;
 } else {
-return (diff > ctx->similarity) ? 255 : 0;
+return (diff > ctx->similarity) ? a : 0;
 }
 }
 
-static uint16_t do_chromakey_pixel16(ChromakeyContext *ctx, uint16_t u[9], 
uint16_t v[9])
+static uint16_t do_chromakey_pixel16(ChromakeyContext *ctx, uint16_t a, 
uint16_t u[9], uint16_t v[9])
 {
 double max = ctx->max;
 double diff = 0.0;
@@ -84,9 +84,9 @@ static uint16_t do_chromakey_pixel16(ChromakeyContext *ctx, 
uint16_t u[9], uint1
 diff /= 9.0;
 
 if (ctx->blend > 0.0001) {
-return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * max;
+return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * 
(float)a;
 } else {
-return (diff > ctx->similarity) ? max : 0;
+return (diff > ctx->similarity) ? a : 0;
 }
 }
 
@@ -131,13 +131,17 @@ static int do_chromakey_slice(AVFilterContext *avctx, 
void *arg, int jobnr, int
 
 for (y = slice_start; y < slice_end; ++y) {
 for (x = 0; x < frame->width; ++x) {
-for (yo = 0; yo < 3; ++yo) {
-for (xo = 0; xo < 3; ++xo) {
-get_pixel_uv(frame, ctx->hsub_log2, ctx->vsub_log2, x + xo 
- 1, y + yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+uint8_t *a = frame->data[3] + frame->linesize[3] * y;
+const uint8_t ao = a[x];
+
+if (ao != 0) {
+for (yo = 0; yo < 3; ++yo) {
+for (xo = 0; xo < 3; ++xo) {
+get_pixel_uv(frame, ctx->hsub_log2, ctx->vsub_log2, x 
+ xo - 1, y + yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+}
 }
+a[x] = do_chromakey_pixel(ctx, ao, u, v);
 }
-
-frame->data[3][frame->linesize[3] * y + x] = 
do_chromakey_pixel(ctx, u, v);
 }
 }
 
@@ -163,15 +167,18 @@ static int do_chromakey16_slice(AVFilterContext *avctx, 
void *arg, int jobnr, in
 
 for (y = slice_start; y < slice_end; ++y) {
 for (x = 0; x < frame->width; ++x) {
-uint16_t *dst = (uint16_t *)(frame->data[3] + frame->linesize[3] * 
y);
-
-for (yo = 0; yo < 3; ++yo) {
-for (xo = 0; xo < 3; ++xo) {
-get_pixel16_uv(frame, ctx->hsub_log2, ctx->vsub_log2, x + 
xo - 1, y + yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+uint16_t *a = (uint16_t *)(frame->data[3] + frame->linesize[3] * 
y);
+const uint16_t ao = a[x];
+
+if (ao != 0) {
+for (yo = 0; yo < 3; ++yo) {
+for (xo = 0; xo < 3; ++xo) {
+get_pixel16_uv(frame, ctx->hsub_log2, ctx->vsub_log2, 
x + xo - 1, y + yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+}
 }
-}
 
-dst[x] = do_chromakey_pixel16(ctx, u, v);
+a[x] = do_chromakey_pixel16(ctx, ao, u, v);
+}
 }
 }
 
-- 
2.17.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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_chromakey: The chromakey filter preserves non-opaque alpha transparency.

2020-06-10 Thread Gavin Smith


On 10/06/2020 22:33, Gavin Smith wrote:

From: Gavin Smith 


Sorry, new to all this. A description would have been helpful. I'll work 
on getting it right.


So this patch is to address Trac ticket #8724:

https://trac.ffmpeg.org/ticket/8724



---
  libavfilter/vf_chromakey.c | 43 ++
  1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/libavfilter/vf_chromakey.c b/libavfilter/vf_chromakey.c
index 4b1669d084..7206f1 100644
--- a/libavfilter/vf_chromakey.c
+++ b/libavfilter/vf_chromakey.c
@@ -47,7 +47,7 @@ typedef struct ChromakeyContext {
  int jobnr, int nb_jobs);
  } ChromakeyContext;
  
-static uint8_t do_chromakey_pixel(ChromakeyContext *ctx, uint8_t u[9], uint8_t v[9])

+static uint8_t do_chromakey_pixel(ChromakeyContext *ctx, uint8_t a, uint8_t 
u[9], uint8_t v[9])
  {
  double diff = 0.0;
  int du, dv, i;
@@ -62,13 +62,13 @@ static uint8_t do_chromakey_pixel(ChromakeyContext *ctx, 
uint8_t u[9], uint8_t v
  diff /= 9.0;
  
  if (ctx->blend > 0.0001) {

-return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * 
255.0;
+return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * 
(float)a;
  } else {
-return (diff > ctx->similarity) ? 255 : 0;
+return (diff > ctx->similarity) ? a : 0;
  }
  }
  
-static uint16_t do_chromakey_pixel16(ChromakeyContext *ctx, uint16_t u[9], uint16_t v[9])

+static uint16_t do_chromakey_pixel16(ChromakeyContext *ctx, uint16_t a, 
uint16_t u[9], uint16_t v[9])
  {
  double max = ctx->max;
  double diff = 0.0;
@@ -84,9 +84,9 @@ static uint16_t do_chromakey_pixel16(ChromakeyContext *ctx, 
uint16_t u[9], uint1
  diff /= 9.0;
  
  if (ctx->blend > 0.0001) {

-return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * max;
+return av_clipd((diff - ctx->similarity) / ctx->blend, 0.0, 1.0) * 
(float)a;
  } else {
-return (diff > ctx->similarity) ? max : 0;
+return (diff > ctx->similarity) ? a : 0;
  }
  }
  
@@ -131,13 +131,17 @@ static int do_chromakey_slice(AVFilterContext *avctx, void *arg, int jobnr, int
  
  for (y = slice_start; y < slice_end; ++y) {

  for (x = 0; x < frame->width; ++x) {
-for (yo = 0; yo < 3; ++yo) {
-for (xo = 0; xo < 3; ++xo) {
-get_pixel_uv(frame, ctx->hsub_log2, ctx->vsub_log2, x + xo - 1, y 
+ yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+uint8_t *a = frame->data[3] + frame->linesize[3] * y;
+const uint8_t ao = a[x];
+
+if (ao != 0) {
+for (yo = 0; yo < 3; ++yo) {
+for (xo = 0; xo < 3; ++xo) {
+get_pixel_uv(frame, ctx->hsub_log2, ctx->vsub_log2, x + xo - 
1, y + yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+}
  }
+a[x] = do_chromakey_pixel(ctx, ao, u, v);
  }
-
-frame->data[3][frame->linesize[3] * y + x] = 
do_chromakey_pixel(ctx, u, v);
  }
  }
  
@@ -163,15 +167,18 @@ static int do_chromakey16_slice(AVFilterContext *avctx, void *arg, int jobnr, in
  
  for (y = slice_start; y < slice_end; ++y) {

  for (x = 0; x < frame->width; ++x) {
-uint16_t *dst = (uint16_t *)(frame->data[3] + frame->linesize[3] * 
y);
-
-for (yo = 0; yo < 3; ++yo) {
-for (xo = 0; xo < 3; ++xo) {
-get_pixel16_uv(frame, ctx->hsub_log2, ctx->vsub_log2, x + xo - 1, 
y + yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+uint16_t *a = (uint16_t *)(frame->data[3] + frame->linesize[3] * 
y);
+const uint16_t ao = a[x];
+
+if (ao != 0) {
+for (yo = 0; yo < 3; ++yo) {
+for (xo = 0; xo < 3; ++xo) {
+get_pixel16_uv(frame, ctx->hsub_log2, ctx->vsub_log2, x + xo - 
1, y + yo - 1, [yo * 3 + xo], [yo * 3 + xo]);
+}
  }
-}
  
-dst[x] = do_chromakey_pixel16(ctx, u, v);

+a[x] = do_chromakey_pixel16(ctx, ao, u, v);
+}
  }
  }
  

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".