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 657319 - videorate should use basetransform
videorate should use basetransform
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.36
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 657318
Blocks:
 
 
Reported: 2011-08-25 10:52 UTC by Sjoerd Simons
Modified: 2011-10-29 15:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (24.04 KB, patch)
2011-08-25 10:52 UTC, Sjoerd Simons
none Details | Review
Additional patch to fix some other issues (3.96 KB, patch)
2011-08-25 14:42 UTC, Sjoerd Simons
accepted-commit_now Details | Review
Updated patch for new basetransform api (24.06 KB, patch)
2011-08-26 14:18 UTC, Sjoerd Simons
needs-work Details | Review
updated patch (24.03 KB, patch)
2011-08-31 13:04 UTC, Sjoerd Simons
none Details | Review

Description Sjoerd Simons 2011-08-25 10:52:16 UTC
Created attachment 194693 [details] [review]
proposed patch

Currently videorate doesn';t support interesting features like forwarding pad-allocs and upstream negotiation, easiest way to fix this is to port to basetransform...
Comment 1 Sjoerd Simons 2011-08-25 14:42:34 UTC
Created attachment 194706 [details] [review]
Additional patch to fix some other issues
Comment 2 Sjoerd Simons 2011-08-26 14:18:03 UTC
Created attachment 194839 [details] [review]
Updated patch for new basetransform api
Comment 3 David Schleef 2011-08-27 06:38:20 UTC
Does basetransform work correctly when input and output buffers are not 1-1?  I'm pretty sure the answer is (was?) no.
Comment 4 Sebastian Dröge (slomo) 2011-08-30 15:01:44 UTC
Review of attachment 194839 [details] [review]:

Looks good in general (and you can use basetransform for non-1-to-1 elements, see audiofirfilter for example, you just have to handle additional buffers yourself and drop buffers with GST_BASE_TRANSFORM_FLOW_DROPPED)

::: gst/videorate/gstvideorate.c
@@ +285,3 @@
 {
+  GstStructure *s;
+  gint nom, denom;

It's numerator, not nomerator/nominator ;)

@@ +302,3 @@
   GstStructure *structure;
   gboolean ret = TRUE;
+  //GstPad *otherpad, *opeer;

C++/C99 comment

@@ -407,3 @@
-      if (gst_structure_has_field (structure, "pixel-aspect-ratio"))
-        gst_structure_fixate_field_nearest_fraction (structure,
-            "pixel-aspect-ratio", 1, 1);

These fixations might still be necessary in the fixate vfunc
Comment 5 Sebastian Dröge (slomo) 2011-08-30 15:04:38 UTC
Review of attachment 194706 [details] [review]:

Looks good
Comment 6 Sjoerd Simons 2011-08-31 13:03:35 UTC
(In reply to comment #4)
> Review of attachment 194839 [details] [review]:
> 
> Looks good in general (and you can use basetransform for non-1-to-1 elements,
> see audiofirfilter for example, you just have to handle additional buffers
> yourself and drop buffers with GST_BASE_TRANSFORM_FLOW_DROPPED)
> 
> ::: gst/videorate/gstvideorate.c
> @@ +285,3 @@
>  {
> +  GstStructure *s;
> +  gint nom, denom;
> 
> It's numerator, not nomerator/nominator ;)
> 
> @@ +302,3 @@
>    GstStructure *structure;
>    gboolean ret = TRUE;
> +  //GstPad *otherpad, *opeer;
> 
> C++/C99 comment

Fixed up/merged this two (will attach the updated patch)

> @@ -407,3 @@
> -      if (gst_structure_has_field (structure, "pixel-aspect-ratio"))
> -        gst_structure_fixate_field_nearest_fraction (structure,
> -            "pixel-aspect-ratio", 1, 1);
> 
> These fixations might still be necessary in the fixate vfunc

As discussed on IRC, basetransform falls back to gst_pad_fixate_caps if after calling the fixate vfunc some fields aren't fixated yet, so this shouldn't be needed anymore
Comment 7 Sjoerd Simons 2011-08-31 13:04:42 UTC
Created attachment 195291 [details] [review]
updated patch
Comment 8 Sjoerd Simons 2011-08-31 13:15:02 UTC
his problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.

commit ea46b3c706ea99257d2b31cb5c7fb281f43b3178
Author: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
Date:   Tue Aug 23 10:11:52 2011 +0200

    videorate: Port to basetransform