After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723271 - videotestsrc: fix a warning if downstream does not propose a buffer pool
videotestsrc: fix a warning if downstream does not propose a buffer pool
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.x
Other All
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-01-30 10:21 UTC by Julien Isorce
Modified: 2014-01-31 14:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videotestsrc: ensure having caps when setting the buffer pool config (1.33 KB, patch)
2014-01-30 10:21 UTC, Julien Isorce
committed Details | Review

Description Julien Isorce 2014-01-30 10:21:58 UTC
Created attachment 267620 [details] [review]
videotestsrc: ensure having caps when setting the buffer pool config

* step to reproduce:

GST_DEBUG=2 gst-launch-1.0 videotestsrc ! fakesink

* actual result:

0:00:00.087664559  7697      0x14f8630 WARN                 default gstvideopool.c:171:video_buffer_pool_set_config:<videobufferpool0> no caps in config
Comment 1 Thiago Sousa Santos 2014-01-30 16:04:01 UTC
Review of attachment 267620 [details] [review]:

::: gst/videotestsrc/gstvideotestsrc.c
@@ -651,1 +652,5 @@
   config = gst_buffer_pool_get_config (pool);
+
+  gst_query_parse_allocation (query, &caps, NULL);
+  if (caps)
+    gst_buffer_pool_config_set_params (config, caps, size, min, max);

How about moving this inside the if(pool == NULL) condition?

AFAIU if the pool was created downstream it should already contain some configuration parameters, including the caps.
Comment 2 Julien Isorce 2014-01-31 09:57:57 UTC
(In reply to comment #1)
> Review of attachment 267620 [details] [review]:
> 
> ::: gst/videotestsrc/gstvideotestsrc.c
> @@ -651,1 +652,5 @@
>    config = gst_buffer_pool_get_config (pool);
> +
> +  gst_query_parse_allocation (query, &caps, NULL);
> +  if (caps)
> +    gst_buffer_pool_config_set_params (config, caps, size, min, max);
> 
> How about moving this inside the if(pool == NULL) condition?
> 
> AFAIU if the pool was created downstream it should already contain some
> configuration parameters, including the caps.

Hi Thiago, thx for your point of view. Actually I thought about it but I think the downstream can also forget to do it.

Hi Sebastian, you have accepted the commit but maybe you missed #1 comment ?

Let me know and then I can push it.
Comment 3 Sebastian Dröge (slomo) 2014-01-31 13:29:10 UTC
Please push it, I don't think you can assume that downstream configures the caps or anything else on the buffer. Especially some configuration can't be done by downstream in every case.
Comment 4 Julien Isorce 2014-01-31 14:11:49 UTC
Comment on attachment 267620 [details] [review]
videotestsrc: ensure having caps when setting the buffer pool config

commit c9b493b853e2c28ad0090b063acf88ae97f080a6
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Fri Jan 31 13:40:36 2014 +0000

    videotestsrc: ensure having caps when setting the buffer pool config
    
    It happens if downstream does not propose a buffer pool.
    GST_DEBUG=2 gst-launch-1.0 videotestsrc ! fakesink
    
    https://bugzilla.gnome.org/show_bug.cgi?id=723271