GStreamer-devel

Re: gst-plugins-bad: dvdspu: Fix pad templates

classic Classic list List threaded Threaded
♦
♦
 5 messages Options
Tim-Philipp Müller-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: gst-plugins-bad: dvdspu: Fix pad templates

Tim-Philipp Müller-2
On Sun, 2011-05-29 at 09:18 -0700, Edward Hervey wrote:

Hi,

> Module: gst-plugins-bad
> Branch: master
> Commit: 732828e31cbace59e6ce4f262b8339e43ed0c631
> URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=732828e31cbace59e6ce4f262b8339e43ed0c631
>
> Author: Edward Hervey <[hidden email]>
> Date:   Sun May 29 18:16:49 2011 +0200
>
> dvdspu: Fix pad templates
>
> Our caps intersection code is a bit too touchy about what an element
> returns compared to its pad templates.

What was the exact problem here? The change doesn't look like it should
be necessary at first glance: additional optional fields in the actual
caps shouldn't be a problem, and neither should the single-value list.

I think there may have been a regression with caps subset/intersection
handling in git core. I've seen a few other weird issues and
not-negotiated errors lately (but haven't investigated yet).

Cheers
 -Tim

> ---
>
>  gst/dvdspu/gstdvdspu.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gst/dvdspu/gstdvdspu.c b/gst/dvdspu/gstdvdspu.c
> index 89b63d8..a68cdd5 100644
> --- a/gst/dvdspu/gstdvdspu.c
> +++ b/gst/dvdspu/gstdvdspu.c
> @@ -55,16 +55,18 @@ static GstStaticPadTemplate video_sink_factory =
>  GST_STATIC_PAD_TEMPLATE ("video",
>      GST_PAD_SINK,
>      GST_PAD_ALWAYS,
> -    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc) { I420 }, "
> -        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ]")
> +    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc)I420, "
> +        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ],"
> +        "framerate = " GST_VIDEO_FPS_RANGE)
>      /* FIXME: Can support YV12 one day too */
>      );
>  
>  static GstStaticPadTemplate src_factory = GST_STATIC_PAD_TEMPLATE ("src",
>      GST_PAD_SRC,
>      GST_PAD_ALWAYS,
> -    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc) { I420 }, "
> -        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ]")
> +    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc)I420, "
> +        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ],"
> +        "framerate = " GST_VIDEO_FPS_RANGE)
>      /* FIXME: Can support YV12 one day too */
>      );
>  
>
> _______________________________________________
> gstreamer-commits mailing list
> [hidden email]
> http://lists.freedesktop.org/mailman/listinfo/gstreamer-commits


_______________________________________________
gstreamer-devel mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel
Edward Hervey
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: gst-plugins-bad: dvdspu: Fix pad templates

Edward Hervey
Administrator
On Sun, 2011-05-29 at 19:02 +0100, Tim-Philipp Müller wrote:

> On Sun, 2011-05-29 at 09:18 -0700, Edward Hervey wrote:
>
> Hi,
>
> > Module: gst-plugins-bad
> > Branch: master
> > Commit: 732828e31cbace59e6ce4f262b8339e43ed0c631
> > URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=732828e31cbace59e6ce4f262b8339e43ed0c631
> >
> > Author: Edward Hervey <[hidden email]>
> > Date:   Sun May 29 18:16:49 2011 +0200
> >
> > dvdspu: Fix pad templates
> >
> > Our caps intersection code is a bit too touchy about what an element
> > returns compared to its pad templates.
>
> What was the exact problem here? The change doesn't look like it should
> be necessary at first glance: additional optional fields in the actual
> caps shouldn't be a problem, and neither should the single-value list.
>
> I think there may have been a regression with caps subset/intersection
> handling in git core. I've seen a few other weird issues and
> not-negotiated errors lately (but haven't investigated yet).

  I thought it was an issue noone had seen before and was specific to
that element.

  It does fail indeed with other caps:

  c1 = gst.Caps("blah,something={1}")
  c2 = gst.Caps("blah,something=2")

  c1.is_subset(c2) => FALSE
  c2.is_subset(c1) => FALSE
  c1.intersect(c2) => empty caps
 

   This commit can be reverted once core is fixed.

    Edward

>
> Cheers
>  -Tim
>
> > ---
> >
> >  gst/dvdspu/gstdvdspu.c |   10 ++++++----
> >  1 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/gst/dvdspu/gstdvdspu.c b/gst/dvdspu/gstdvdspu.c
> > index 89b63d8..a68cdd5 100644
> > --- a/gst/dvdspu/gstdvdspu.c
> > +++ b/gst/dvdspu/gstdvdspu.c
> > @@ -55,16 +55,18 @@ static GstStaticPadTemplate video_sink_factory =
> >  GST_STATIC_PAD_TEMPLATE ("video",
> >      GST_PAD_SINK,
> >      GST_PAD_ALWAYS,
> > -    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc) { I420 }, "
> > -        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ]")
> > +    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc)I420, "
> > +        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ],"
> > +        "framerate = " GST_VIDEO_FPS_RANGE)
> >      /* FIXME: Can support YV12 one day too */
> >      );
> >  
> >  static GstStaticPadTemplate src_factory = GST_STATIC_PAD_TEMPLATE ("src",
> >      GST_PAD_SRC,
> >      GST_PAD_ALWAYS,
> > -    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc) { I420 }, "
> > -        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ]")
> > +    GST_STATIC_CAPS ("video/x-raw-yuv, " "format = (fourcc)I420, "
> > +        "width = (int) [ 16, 4096 ], " "height = (int) [ 16, 4096 ],"
> > +        "framerate = " GST_VIDEO_FPS_RANGE)
> >      /* FIXME: Can support YV12 one day too */
> >      );
> >  
> >
> > _______________________________________________
> > gstreamer-commits mailing list
> > [hidden email]
> > http://lists.freedesktop.org/mailman/listinfo/gstreamer-commits
>
>


_______________________________________________
gstreamer-devel mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel
Sebastian Dröge-7
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: gst-plugins-bad: dvdspu: Fix pad templates

Sebastian Dröge-7
On Sun, 2011-05-29 at 23:05 +0200, Edward Hervey wrote:

> On Sun, 2011-05-29 at 19:02 +0100, Tim-Philipp Müller wrote:
> > On Sun, 2011-05-29 at 09:18 -0700, Edward Hervey wrote:
> >
> > Hi,
> >
> > > Module: gst-plugins-bad
> > > Branch: master
> > > Commit: 732828e31cbace59e6ce4f262b8339e43ed0c631
> > > URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=732828e31cbace59e6ce4f262b8339e43ed0c631
> > >
> > > Author: Edward Hervey <[hidden email]>
> > > Date:   Sun May 29 18:16:49 2011 +0200
> > >
> > > dvdspu: Fix pad templates
> > >
> > > Our caps intersection code is a bit too touchy about what an element
> > > returns compared to its pad templates.
> >
> > What was the exact problem here? The change doesn't look like it should
> > be necessary at first glance: additional optional fields in the actual
> > caps shouldn't be a problem, and neither should the single-value list.
> >
> > I think there may have been a regression with caps subset/intersection
> > handling in git core. I've seen a few other weird issues and
> > not-negotiated errors lately (but haven't investigated yet).
>
>   I thought it was an issue noone had seen before and was specific to
> that element.
>
>   It does fail indeed with other caps:
>
>   c1 = gst.Caps("blah,something={1}")
>   c2 = gst.Caps("blah,something=2")
>
>   c1.is_subset(c2) => FALSE
>   c2.is_subset(c1) => FALSE
>   c1.intersect(c2) => empty caps
>  
>
>    This commit can be reverted once core is fixed.
That's probably my fault then and I'll try to fix it after understanding
what exactly is wrong.

In the example you give above, why do you think this is wrong though?
These two caps are completely distinct, none is a subset of the other
and the intersection should be empty because both are incompatible. What
am I missing? :)

_______________________________________________
gstreamer-devel mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

signature.asc (205 bytes) Download Attachment
Sebastian Dröge-7
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: gst-plugins-bad: dvdspu: Fix pad templates

Sebastian Dröge-7
On Mon, 2011-05-30 at 04:01 +0200, Sebastian Dröge wrote:

> On Sun, 2011-05-29 at 23:05 +0200, Edward Hervey wrote:
> > On Sun, 2011-05-29 at 19:02 +0100, Tim-Philipp Müller wrote:
> > > On Sun, 2011-05-29 at 09:18 -0700, Edward Hervey wrote:
> > >
> > > Hi,
> > >
> > > > Module: gst-plugins-bad
> > > > Branch: master
> > > > Commit: 732828e31cbace59e6ce4f262b8339e43ed0c631
> > > > URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=732828e31cbace59e6ce4f262b8339e43ed0c631
> > > >
> > > > Author: Edward Hervey <[hidden email]>
> > > > Date:   Sun May 29 18:16:49 2011 +0200
> > > >
> > > > dvdspu: Fix pad templates
> > > >
> > > > Our caps intersection code is a bit too touchy about what an element
> > > > returns compared to its pad templates.
> > >
> > > What was the exact problem here? The change doesn't look like it should
> > > be necessary at first glance: additional optional fields in the actual
> > > caps shouldn't be a problem, and neither should the single-value list.
> > >
> > > I think there may have been a regression with caps subset/intersection
> > > handling in git core. I've seen a few other weird issues and
> > > not-negotiated errors lately (but haven't investigated yet).
> >
> >   I thought it was an issue noone had seen before and was specific to
> > that element.
> >
> >   It does fail indeed with other caps:
> >
> >   c1 = gst.Caps("blah,something={1}")
> >   c2 = gst.Caps("blah,something=2")
> >
> >   c1.is_subset(c2) => FALSE
> >   c2.is_subset(c1) => FALSE
> >   c1.intersect(c2) => empty caps
> >  
> >
> >    This commit can be reverted once core is fixed.
>
> That's probably my fault then and I'll try to fix it after understanding
> what exactly is wrong.
>
> In the example you give above, why do you think this is wrong though?
> These two caps are completely distinct, none is a subset of the other
> and the intersection should be empty because both are incompatible. What
> am I missing? :)
Oh I see, you wanted to use {1} and 1, not 2. Yes, this is broken
currently, will fix it...

_______________________________________________
gstreamer-devel mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

signature.asc (205 bytes) Download Attachment
Edward Hervey
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: gst-plugins-bad: dvdspu: Fix pad templates

Edward Hervey
Administrator
On Mon, 2011-05-30 at 06:46 +0200, Sebastian Dröge wrote:

> On Mon, 2011-05-30 at 04:01 +0200, Sebastian Dröge wrote:
> > On Sun, 2011-05-29 at 23:05 +0200, Edward Hervey wrote:
> > >   It does fail indeed with other caps:
> > >
> > >   c1 = gst.Caps("blah,something={1}")
> > >   c2 = gst.Caps("blah,something=2")
> > >
> > >   c1.is_subset(c2) => FALSE
> > >   c2.is_subset(c1) => FALSE
> > >   c1.intersect(c2) => empty caps
> > >  
> > >
> > >    This commit can be reverted once core is fixed.
> >
> > That's probably my fault then and I'll try to fix it after understanding
> > what exactly is wrong.
> >
> > In the example you give above, why do you think this is wrong though?
> > These two caps are completely distinct, none is a subset of the other
> > and the intersection should be empty because both are incompatible. What
> > am I missing? :)
>
> Oh I see, you wanted to use {1} and 1, not 2. Yes, this is broken
> currently, will fix it...

  Thanks for spotting my stupid sunday afternoon typo. Yes, I indeed
meant 1 and not 2.

  Will revert my other patch.

    Edward

_______________________________________________
gstreamer-devel mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel
Free forum by Nabble Edit this page