| Message ID | 20170721095003.20505-1-george@nsup.org |
|---|---|
| State | Accepted |
| Commit | bbc7cfbf1e0b02323d4af512612342d2627080d8 |
| Headers | show |
2017-07-21 11:50 GMT+02:00 Nicolas George <george@nsup.org>: > Note: until now, the "default" was 0, not 255. But it produces an ugly > image, so I considered nobody really uses it like that and decided it was ok > to change it to a saner default, but it can be discussed. It does not break > our existing FATE tests. The default you suggest is of course much better than the original default but ideally, some kind of "banding" should be used to allow testing transparency in libswscale, encoders and decoders. Carl Eugen
Le tridi 3 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : > The default you suggest is of course much better than the original > default but ideally, some kind of "banding" should be used to allow > testing transparency in libswscale, encoders and decoders. Can you be a little more specific? I am not sure I understand what you mean exactly. Would having two different alpha values for the alternating vertical bands fill your suggestion? Regards,
2017-07-21 15:40 GMT+02:00 Nicolas George <george@nsup.org>: > Le tridi 3 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : >> The default you suggest is of course much better than the original >> default but ideally, some kind of "banding" should be used to allow >> testing transparency in libswscale, encoders and decoders. > > Can you be a little more specific? I am not sure I understand what > you mean exactly. I meant value = width or value = height (but I am not saying this is necessarily a wise suggestion). > Would having two different alpha values for the > alternating vertical bands fill your suggestion? I think so. Thank you, Carl Eugen
Le tridi 3 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : > I meant value = width or value = height (but I am not saying this > is necessarily a wise suggestion). I am sorry, but now I really do not understand at all. Regards,
2017-07-21 15:43 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > 2017-07-21 15:40 GMT+02:00 Nicolas George <george@nsup.org>: >> Le tridi 3 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : >>> The default you suggest is of course much better than the original >>> default but ideally, some kind of "banding" should be used to allow >>> testing transparency in libswscale, encoders and decoders. >> >> Can you be a little more specific? I am not sure I understand what >> you mean exactly. > > I meant value = width or value = height alpha_value = x * 255 / width; or alpha_value = y * 255 / height; Sorry for being unclear, Carl Eugen
Le tridi 3 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : > alpha_value = x * 255 / width; > or > alpha_value = y * 255 / height; > > Sorry for being unclear, Carl Eugen Oh, you mean a gradient on the alpha channel. It is not really possible without making the code more complex and I think significantly slower. A coarse gradient between the different vertical bands is easy, but the alpha must be constant in each band, just like the color. A gradient on the undulating diagonal line is possible and easy, OTOH, since it already has a color gradient. What do you think? Regards,
2017-07-21 15:51 GMT+02:00 Nicolas George <george@nsup.org>: > Le tridi 3 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : >> alpha_value = x * 255 / width; >> or >> alpha_value = y * 255 / height; >> >> Sorry for being unclear, Carl Eugen > > Oh, you mean a gradient on the alpha channel. It is not really possible > without making the code more complex and I think significantly slower. > A coarse gradient between the different vertical bands is easy, but the > alpha must be constant in each band, just like the color. (Why?) > A gradient on the undulating diagonal line is possible and easy, OTOH, > since it already has a color gradient. Any change that provides different transparency values on one frame is a useful fix for a long-standing bug / missing feature imo. Thank you, Carl Eugen
Le sextidi 6 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : > > Oh, you mean a gradient on the alpha channel. It is not really possible > > without making the code more complex and I think significantly slower. > > > A coarse gradient between the different vertical bands is easy, but the > > alpha must be constant in each band, just like the color. > > (Why?) Because memset() is faster and simpler than anything else that could do the work. testsrc2 is based on the drawutils functions, which provide a few drawing primitives, especially rectangles. They are limited in features but are fast and support many pixel formats with little code. > Any change that provides different transparency values on one frame is > a useful fix for a long-standing bug / missing feature imo. Note that with the current patch, while most of the drawing has constant alpha, the beating cross and bouncing square ball are fully opaque, so there are already two alpha values. It is better than nothing. I suggest we apply the patch as is, since it allows the overlay test to move forward. And then we can consider how to add an interesting alpha channel for better testing. Is it ok with you? For the alpha, I suggest the following: Somebody should implement an animated plasma (fractal) generator. I have been meaning to do that for some time, but I have not yet found an elegant and efficient way to dot, and copying somebody else's algorithm when I think I can devise my own is not my style. If that happens, we can use a gray plasma as the alpha channel of testsrc2 for planar modes, either with the alphamerge filter or with easy additional code. It would be a matter of minutes to implement that as an option. For the plasma generator, my idea is to use a deterministic 3D pink-ish noise generator: the screen is a slice of a x-y plane at z=PTS. For the pink-ish noise, I consider the following algorithm: Select random values at a certain interval/frequency, and make a linear interpolation between, that yields a random zigzag signal. Do the same at half interval / double frequency, quarter interval / quadruple frequency, etc., until the sampling precision is reached, and add all the random zigzags together. The relative amplitudes of the frequency bands determine the color of the noise: constant amplitude over the audible frequencies yields a rather good pink noise. Maybe, on top of that, apply soft-clipping to be able to generate a signal with a decent amplitude without clipping. For multidimensional plasma, instead of zigzaging between random values at regular interval, it must select random values on a regular grid and interpolate with the multilinear fit. But because of that, the values cannot be selected in order and then discarded, it requires either caching the values or re-generating them deterministically. If somebody has a very fast parametric PRNG to suggest, it would be helpful. By parametric, I mean: rand(x,y,z,f) -> a number, always the same for the same values of (x,y,z,f) but with all the properties of a PRNG as soon as any parameter changes. Anyway, if anybody wants to implement a plasma or any kind of deterministic nice-looking animated gradient, it would be helpful for the project. Regards,
2017-07-24 13:02 GMT+02:00 Nicolas George <george@nsup.org>: > Le sextidi 6 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : >> > Oh, you mean a gradient on the alpha channel. It is not really possible >> > without making the code more complex and I think significantly slower. >> >> > A coarse gradient between the different vertical bands is easy, but the >> > alpha must be constant in each band, just like the color. >> >> (Why?) > > Because memset() is faster and simpler than anything else that could do > the work. I thought that one of my suggestions (alpha_value = y * 255 / height) would play nice with memset(). > testsrc2 is based on the drawutils functions, which provide a > few drawing primitives, especially rectangles. They are limited in > features but are fast and support many pixel formats with little code. > >> Any change that provides different transparency values on one frame is >> a useful fix for a long-standing bug / missing feature imo. > > Note that with the current patch, while most of the drawing has constant > alpha, the beating cross and bouncing square ball are fully opaque, so > there are already two alpha values. It is better than nothing. Definitely. > I suggest we apply the patch as is, since it allows the overlay test to > move forward. And then we can consider how to add an interesting alpha > channel for better testing. > Is it ok with you? I am ok with everything that improves the current situation, I just wanted to point out a long-standing issue when I saw your patch. I cannot comment on the Plasma generator. Carl Eugen
Le septidi 7 thermidor, an CCXXV, Carl Eugen Hoyos a écrit : > I thought that one of my suggestions (alpha_value = y * 255 / height) > would play nice with memset(). Yes, indeed, a vertical gradient would play nicer. I keep it in mind for the future. > I am ok with everything that improves the current situation, I just > wanted to point out a long-standing issue when I saw your patch. Thanks, pushed. Regards,
diff --git a/doc/filters.texi b/doc/filters.texi index 2708fdb1df..e265c4ef59 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -16390,6 +16390,11 @@ The sources accept the following parameters: @table @option +@item alpha +Specify the alpha (opacity) of the background, only available in the +@code{testsrc2} source. The value must be between 0 (fully transparent) and +255 (fully opaque, the default). + @item color, c Specify the color of the source, only available in the @code{color} source. For the syntax of this option, check the "Color" section in the diff --git a/libavfilter/vsrc_testsrc.c b/libavfilter/vsrc_testsrc.c index c4a5ae3742..fe0d50aa41 100644 --- a/libavfilter/vsrc_testsrc.c +++ b/libavfilter/vsrc_testsrc.c @@ -66,6 +66,9 @@ typedef struct TestSourceContext { /* only used by testsrc */ int nb_decimals; + /* only used by testsrc2 */ + int alpha; + /* only used by color */ FFDrawContext draw; FFDrawColor color; @@ -685,6 +688,7 @@ AVFilter ff_vsrc_testsrc = { static const AVOption testsrc2_options[] = { COMMON_OPTIONS + { "alpha", "set global alpha (opacity)", OFFSET(alpha), AV_OPT_TYPE_INT, {.i64 = 255}, 0, 255, FLAGS }, { NULL } }; @@ -735,6 +739,7 @@ static void test2_fill_picture(AVFilterContext *ctx, AVFrame *frame) { TestSourceContext *s = ctx->priv; FFDrawColor color; + unsigned alpha = (uint32_t)s->alpha << 24; /* colored background */ { @@ -746,7 +751,8 @@ static void test2_fill_picture(AVFilterContext *ctx, AVFrame *frame) x2 = ff_draw_round_to_sub(&s->draw, 0, 0, x2); set_color(s, &color, ((i & 1) ? 0xFF0000 : 0) | ((i & 2) ? 0x00FF00 : 0) | - ((i & 4) ? 0x0000FF : 0)); + ((i & 4) ? 0x0000FF : 0) | + alpha); ff_fill_rectangle(&s->draw, &color, frame->data, frame->linesize, x, 0, x2 - x, frame->height); x = x2; @@ -763,7 +769,7 @@ static void test2_fill_picture(AVFilterContext *ctx, AVFrame *frame) g0 = av_rescale_q(s->pts, s->time_base, av_make_q(1, 128)); for (x = 0; x < s->w; x += dx) { g = (av_rescale(x, 6 * 256, s->w) + g0) % (6 * 256); - set_color(s, &color, color_gradient(g)); + set_color(s, &color, color_gradient(g) | alpha); y = y0 + av_rescale(x, s->h / 2, s->w); y %= 2 * (s->h - 16); if (y > s->h - 16) @@ -785,7 +791,7 @@ static void test2_fill_picture(AVFilterContext *ctx, AVFrame *frame) int c, i; for (c = 0; c < 3; c++) { - set_color(s, &color, 0xBBBBBB ^ (0xFF << (c << 3))); + set_color(s, &color, (0xBBBBBB ^ (0xFF << (c << 3))) | alpha); pos = av_rescale_q(s->pts, s->time_base, av_make_q(64 >> (c << 1), cycle)) % cycle; xh = pos < 1 * l ? pos : pos < 2 * l ? l :
Signed-off-by: Nicolas George <george@nsup.org> --- doc/filters.texi | 5 +++++ libavfilter/vsrc_testsrc.c | 12 +++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) Fixed the undefined shift, thanks Clément for noticing. Added documentation. Note: until now, the "default" was 0, not 255. But it produces an ugly image, so I considered nobody really uses it like that and decided it was ok to change it to a saner default, but it can be discussed. It does not break our existing FATE tests. Note to whoever will push "fate: add vf_overlay test for main source with alpha channel": if this patch is required and I have not applied it, feel free to do it yourself.