Created attachment 258081 [details] [review]
0001-ivfparse-port-to-1.x-api.patch

Created attachment 258082 [details] [review]
0002-configure-build-ivfparse-after-1.x-proting.patch

Review of attachment 258081 [details] [review]:

::: gst/ivfparse/gstivfparse.c
@@ +219,3 @@
+        segment.start = 0;
+        segment.stop = -1;
+        segment.position = 0;

These are the default values of the segment after gst_segment_init()

@@ +220,3 @@
+        segment.stop = -1;
+        segment.position = 0;
+        gst_pad_push_event (ivf->srcpad, gst_event_new_segment (&segment));

Additionally you will need to implement a event function on the sinkpad and drop the segment event received there

Comment on attachment 258082 [details] [review]
0002-configure-build-ivfparse-after-1.x-proting.patch

typo: proting should be porting

Created attachment 259522 [details] [review]
0001-ivfparse-port-to-1.x-api

Created attachment 259523 [details] [review]
0002-configure-build-ivfparse-after-1.x-porting

thanks slomo, updated patches attached.

Review of attachment 259522 [details] [review]:

::: gst/ivfparse/gstivfparse.c
@@ +219,3 @@
 
+        gst_segment_init (&segment, GST_FORMAT_TIME);
+        gst_pad_push_event (ivf->srcpad, gst_event_new_segment (&segment));

You first need to send a stream-start event, then the caps event, then the segment event. You don't send a stream-start even currently.

@@ +234,3 @@
+        guint8 data[12];
+
+        gst_adapter_copy (ivf->adapter, data, 0, 12);

Why do you here use copy/flush, and above use take_bytes?

could you point me a link on how to deal with these gst event? I know few about it. thanks

i use gst_adapter_copy/flush, because the 12 bytes is flushed only after the adapter has the complete frame. otherwise it does nothing.

This should be ported to GstBaseParse

Created attachment 272319 [details] [review]
ivfparse: port to baseparse

This patch adds support for the new GstBaseParse APIs to the IVF video parser. Tested ivfparse ! { vp8dec, vaapidecode }, no difference in conformance testing and additional streams.

Except that, in both cases, 03-segmentation-1436 and 03-segmentation-1425 fail because resolutions changes are not supported yet. Though, this is not a parser problem since the format is pretty simple and doesn't allow for that anyway. i.e. this is a decoder side bug.

Sorry, I had tried to keep most of the original code and structure, but the resulting patch doesn't look so nice. You could locally apply and git show -w instead to ease reading. :)

Ping. If nobody objects, I will push this one with the codecparsers change. I have been using that with no issue so far. wrt. resolution change, this is no longer an issue from vaapidecode point of view since it now handles that case. However, from a video parser POV, is it usual to track resolution changes and propagate this to src pad GstCaps? If so, I will write a subsequent patch that uses the new VP8 codecparsers. Thanks.

> wrt. resolution change, this is no longer an issue from vaapidecode point of
> view since it now handles that case. However, from a video parser POV, is
> it usual to track resolution changes and propagate this to src pad GstCaps?

Yes it is, but it's not really a blocker for pushing the initial patch IMHO.

commit abadffd4d848860ff19d4545502fc85f473a5a46
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Fri Apr 18 18:12:36 2014 +0200

    ivfparse: detect and propagate resolution changes.
    
    Detect resolution changes on key frames, and propagate the resulting
    caps to the src pad. Only the uncompressed data chunk is decoded, so
    avoid using the new VP8 bitstream parsing library for now.
    
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>

commit 3b308cba3c6e5bfe2dbfc1a1f83640b44a27e5f9
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Fri Apr 18 17:34:08 2014 +0200

    ivfparse: avoid possible division-by-zero when calculating PTS.
    
    Avoid possible division-by-zero while deriving the presentation timestamp
    of the buffer. The base class will take care of any interpolation needs.
    
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>

commit 4cf290074fd000ed882f1c263ab834df8472651e
Author: Halley Zhao <halley.zhao@intel.com>
Date:   Fri Oct 25 07:38:53 2013 +0800

    ivfparse: enable build.
    
    Drop `ivfparse' element from the non-ported set of plugins in configure.

commit 3d0ce67fcd886cf6e03f4f123d7da64491b96a8d
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Tue Mar 4 15:46:58 2014 +0100

    ivfparse: port to baseparse.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=710855
    
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>

I have pushed the patch to git master branch. I would like to backport to 1.2 series. OK?

It should be enough to cherry-pick everything as I finally did not use the VP8 bitstream parser. i.e. only the uncompressed data chunk was needed, so I manually parsed the necessary bits from there. When VP8 parser meta are available, we could use the codecparser in ivfparse then.

Yes, feel free to cherry pick the port into 1.2, either as one single merged commit or multiple commits.