Created attachment 99763 [details] [review]
reduce CPU usage for unicolor buffers

The image rendering is done for every frame on purpose, so that if a downstream element stomps on the buffer (which it isn't supposed to, but...) it doesn't affect future buffers.

Also, this patch does not appear to handle changing properties correctly.  It also will have problems after renegotiating caps.

Closing this as WONTFIX as the behaviour is correct.

Not sure if WONTFIX is the right resolution.
Having a static-red, static-black et.c variants would be nice (does not need to be all patterns). This comes handy in testing/profiling elements.

Laurent, would you be willing to rework the patch to add the static ones as new patterns (and fix it so that it handles renegotiation)? If so, please reopen.

this check is too weak:
if (v->unicolor_buffer == NULL || (v->unicolor_buffer_size != ((guint) (p->endptr - p->yp)))) {

if the yuv format changes, the size might still be the same.

Sorry, I'm reopening this. The reason stated by ds in Comment #2 is unacceptable.

If some elements write in read-only buffers... those elements should be fixed.

That being said, the patch isn't good either.

I too think the current behaviour is desirable as default, but wouldn't object if a property was added that allows buffer re-use and avoids excessive repainting.

It's not videotestsrc's job to create output that might expose bugs in elements, its job is to create as perfect a signal/stream as possible, and this includes outputting buffers with freshly allocated data for each buffer IMHO.

FWIW, I don't necessarily think the current behavior is correct, I was just giving the reason why it was chosen.

The original purpose of videotestsrc was was to create the default pattern in as many formats as possible, and do it *correctly* and *reliably*.  It helped uncover lots of bugs in other elements, in X drivers, etc.  These days, everything else is just as reliable, plus we have a convenience library for video formats.  (Which, btw, videotestsrc does not use, but that's another matter.)  And now vts has lots of patterns and is useful for other purposes, so imo videotestsrc has outgrown that original purpose.

So I'm in favor of fixing up vts for repurposing.  In particular, I like the idea of not regenerating the same buffer.  I also like the idea of generating a scan line and then memcpy'ing a bunch of times for patterns like smpte and checkers-*.  I'm also in favor of using libgstvideo.  And any changes that can make it more Orc friendly.

Reopening since it seems there's interest in it. Will clean-up attach my avoid-regenerating patch here soon.

Created attachment 166393 [details] [review]
videotestsrc: Don't re-render every frame when it's not needed.

For all the formats that support it and whenever downstream hasn't
changed, we returned a metadata-writable read-only copy of the
original buffer.

Can we commit this now?

The zone plate patterns are not static, nor is ball.  Setting horizontal-speed makes most patterns non-static.

I would prefer pattern_is_static to cachable_format, and make the default FALSE.

Otherwise, push it.

I think it still make sense to have this. Especially on embedded to have a low cpu source for testing the rest of the pipeline.

Yes, just needs someone to update the patch and actually do it

*** Bug 754632 has been marked as a duplicate of this bug. ***

Assign to myself, need to refactor it.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/10.