[gst-devel] proposal: support for row-stride in gstreamer
Rob Clark
rob at ti.com
Thu Sep 10 02:36:43 CEST 2009
Just to update.. I have pipelines with rowstride working fine. The
approach is relatively straightforward and not too revolutionary.
Just normal caps negotiation, which I guess should not be too
surprising.
I did add some utility functions and macros in libgstvideo to help
with the caps building/parsing. And I did add a patch on
gst_pad_fixate_caps() (see below). Other than gst_pad_fixate_caps,
the changes are all in gst-plugins-base. You can find my git tree:
git://github.com/robclark/gst-plugins-base.git
http://github.com/robclark/gst-plugins-base/commits/master
(I agree with earlier comments that stride-transformation should be
supported in ffmpegcolorspace.. but this part I've not had a chance
to implement yet.)
------------------------------------------
Essentially what is required is for a video playback pipeline:
1) srcpad on video decoders, sinkpad on video sinks, etc, should add x-
raw-yuv-strided to their template caps:
static GstStaticPadTemplate src_template =
GST_STATIC_PAD_TEMPLATE ("src",
GST_PAD_SRC,
GST_PAD_ALWAYS,
GST_STATIC_CAPS (GST_VIDEO_CAPS_YUV_STRIDED (
"{ I420, YUY2, UYVY }", "[ 0, max ]"))
);
2) elements like the video decoders should implement a _get_caps()
function for the srcpad which returns two equivalent structures, video/
x-raw-yuv and video/x-raw-yuv-strided (the later with a rowstride
field). It might be worthwhile to add a helper function in
libgstvideo for this.
3) And in the _set_caps() function for video decoder srcpad should do
something like:
if (gst_video_format_parse_caps_strided (caps,
&format, &width, &height, &rowstride)) {
... configure decoder width/height/fourcc ...
if (rowstride) {
... configure decoder rowstride ...
}
}
and of course the video decoder should pad_alloc() their buffers to
give the video sink element an opportunity to dictate the rowstride
that it would prefer to use, if any.
4) To have *some* caps when allocating the first buffer, I used
something like:
new_caps = gst_caps_intersect (gst_pad_get_caps (srcpad),
gst_pad_peer_get_caps (srcpad));
if (!gst_caps_is_fixed (new_caps)) {
gst_caps_do_simplify (new_caps);
gst_pad_fixate_caps (srcpad, new_caps);
}
gst_pad_set_caps (srcpad, new_caps);
Now here is where I ran into a small issue that I think is best fixed
in gst_pad_fixate_caps(). If the caps consists of multiple structs
(such as strided and regular non-strided caps), gst_pad_fixate_caps()
will only fixate the individual structs, and not choose a single
struct to make the caps fixed. I made a small change to remove all
but the first struct:
------------------------------------------
gst_pad_fixate_caps (GstPad * pad, GstCaps * caps)
{
GstPadFixateCapsFunction fixatefunc;
- guint n, len;
+ guint len;
g_return_if_fail (GST_IS_PAD (pad));
g_return_if_fail (caps != NULL);
@@ -2359,11 +2359,15 @@ gst_pad_fixate_caps (GstPad * pad, GstCaps *
caps)
/* default fixation */
len = gst_caps_get_size (caps);
- for (n = 0; n < len; n++) {
- GstStructure *s = gst_caps_get_structure (caps, n);
+ if (len > 0) {
+ GstStructure *s = gst_caps_get_structure (caps, 0);
gst_structure_foreach (s, gst_pad_default_fixate, s);
}
+
+ while (len > 1) {
+ gst_caps_remove_structure (caps, --len);
+ }
}
------------------------------------------
I'm curious if this is the correct solution, or if there is some use-
case for a function which fixates the individual structs but does not
make the caps fixed?
BR,
-R
On Aug 11, 2009, at 2:03 PM, Clark, Rob wrote:
> Hi Jun,
>
> I would like to avoid subclassing GstBuffer for this.. since a lot of
> videosink (and other) elements already subclass GstBuffer for various
> purposes. Having to both subclass GstBuffer and GstVideoBuffer would
> make a mess.
>
> If there was some case where the stride could be changing from frame
> to frame, the buffer metadata proposal would solve this:
>
> http://cgit.freedesktop.org/gstreamer/gstreamer/tree/docs/design/draft-buffer2.txt
>
> but, at least in the cases I can think of, the stride would not be
> changing from frame to frame.
>
> I'm currently working on implementing support for it in gst-openmax
> video decoder elements.. which at least should serve as a good
> reference to the changes to make in other video decoder/encoder
> elements. I think it won't be so complicated, although the decoder
> element would need to support re-negotiating the caps when it
> pad_alloc's a buffer from the video sink element. But this shouldn't
> be a big deal.
>
> ----
>
> as far as stridetransform.. I wouldn't expect it to be part of a
> "normal" pipeline. I'm using it for now for testing. If the
> consensus is that normal pipelines might need some stride
> transformation, I think it should be combined with colorspace
> conversion, to avoid multiple passes over the decoded video frame.
> (But I've not had a chance to implement this yet.)
>
> BR,
> -R
>
>
> On Aug 11, 2009, at 12:37 AM, Jim Nie wrote:
>
>> I agree to use x-raw-yuv-stride, x-raw-rgb-stride to mark the
>> buffer and pad capability. However, stridetransform element may
>> impact
>> the other elements and application usage. It will be big effort to
>> adopt it.
>>
>> I have a propose that may not be mature neither. Comments are
>> welcome.
>> Elements that support stride, or sub region of video buffers will
>> support both x-raw-yuv-stride and x-raw-yuv. If successfully
>> negotiated with downstream element, its src_pad will use
>> gst_video_buffer that derived from gst_buffer. gst_video_buffer
>> contains the offset/stride information.
>>
>> If element fails to negotiate with downstream element with
>> x-raw-yuv-stride, it will try x-raw-yuv as current model. Memory is
>> copied from subregion to new malloced memory and push to downstream.
>>
>> In this way, we can keep compatible with current framework. All
>> effort introduced by supporting subregion/stride is limited to the
>> elements that want to support it.
>>
>> Jun
>>
>> 2009/8/4, Rob Clark <rob at ti.com>:
>>>
>>> On Aug 4, 2009, at 4:49 AM, Jan Schmidt wrote:
>>>
>>>> Hi,
>>>>
>>>> On Thu, 2009-07-30 at 11:45 -0500, Rob Clark wrote:
>>>>> so, my next steps (at least my current thinking about next steps)
>>>>> are
>>>>> to start adding some rowstride related stuff in gst-plugins-base:
>>>>>
>>>>>
>>>>> gst-libs/gst/video/video.c:
>>>>> 1) add gst_video_format_get_size_strided()
>>>>> 2) change gst_video_format_parse_caps() to understand strided
>>>>> caps,
>>>>> and add a gst_video_format_parse_caps_strided() function (it
>>>>> seems
>>>>> there are enough places already using
>>>>> gst_video_format_parse_caps()
>>>>> that I probably don't want to change the signature of this
>>>>> function)
>>>>>
>>>>> and then add a GstStrideTransform element under gst/stride
>>>>
>>>> I haven't put enough thought into designing a stride system to say
>>>> anything for certain. Particularly, I'm not sure whether we'll be
>>>> able
>>>> to safely and successfully integrate it into the 0.10 series
>>>> without
>>>> thinking about it harder.
>>>>
>>>> I am pretty sure, however, that adding a separate stride adjust
>>>> element
>>>> is the wrong way to go. None of the existing pipelines will include
>>>> it,
>>>> so it would no be useful without changes to the applications/
>>>> playbin
>>>> etc.
>>>>
>>>> My hunch is that it will be better to add a new 'video/x-raw-yuv-
>>>> full'
>>>> format and add stride as a new parameter, and adjust
>>>> ffmpegcolorspace to
>>>> support conversions to/from the unstrided/strided formats.
>>>
>>>
>>> Hmm.. my idea for stridetransform was mainly just an element for
>>> testing and debugging with manually constructed gst-launch
>>> pipelines.
>>> We need some way to run codecs both with and without rowstide and
>>> verifying the output. For example:
>>>
>>>
>>> ... ! decoder ! video/x-raw-
>>> yuv,format=(fourcc)YUY2,width=320,height=240,framerate=30/1 !
>>> filesink
>>> location=file1.dat
>>>
>>> and then
>>>
>>> ... ! decoder ! video/x-raw-yuv-
>>> strided
>>> ,format
>>> =(fourcc)YUY2,width=320,height=240,rowstride=700,framerate=30/1 !
>>> stridetransform ! video/x-raw-
>>> yuv,format=(fourcc)YUY2,width=320,height=240,framerate=30/1 !
>>> filesink
>>> location=file2.dat
>>>
>>>
>>> But if you think this should be part of a normal pipeline, then I
>>> think it would make sense to merge in with the colorspace
>>> conversion,
>>> so you have one memory copy instead of two.
>>>
>>> But do you think this is required? At least in the cases that I
>>> have
>>> in mind, the video sink will also support non-strided buffers, but
>>> will be falling back to a less optimal mechanism.
>>>
>>>
>>> ------
>>>
>>> btw, at the risk of starting a bikeshed discussion, is 'video/x-raw-
>>> yuv-full' preferred to 'video/x-raw-yuv-strided'? So far I've been
>>> using the latter, but I don't mind changing the code that I've
>>> written
>>> so far. Should the functions added to video.c have the _full suffix
>>> instead of _strided (ie. gst_video_format_parse_caps_full() instead
>>> of
>>> gst_video_format_parse_caps_strided())?
>>>
>>> If we go with -full, are there any other fields we should add to the
>>> caps at the same time? Offhand, the only thing I can think of that
>>> is
>>> a mandatory field would be rowstride, but I haven't thought too much
>>> about, for example, interlaced buffers.
>>>
>>> ------
>>>
>>> fyi, I've begun making some changes in my private tree at
>>> http://github.com/robclark/gst-plugins-base
>>> ... but nothing that I've changed so far is set in stone. But we do
>>> have to start adding support in the codec and other elements that
>>> we'll use pretty soon, so comments and suggestions now are greatly
>>> appreciated. We can stay on our own tree, or a special rowstride
>>> branch, for now if integration to master is post-0.10. But it would
>>> be nice to not have to *completely* re-write things later ;-)
>>>
>>>
>>>
>>> BR,
>>> -R
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-make-gst_pad_fixate_caps-return-fixed-caps-even-if-t.patch
Type: application/octet-stream
Size: 1484 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/gstreamer-devel/attachments/20090909/9663b7db/attachment.obj>
More information about the gstreamer-devel
mailing list