[Bug 671909] Port base video encoding and decoding classes from gst-plugins-bad

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=671909
  GStreamer | gst-plugins-base | git

--- Comment #15 from Edward Hervey <bilboed at gmail.com> 2012-04-04 08:27:11 UTC ---
(In reply to comment #10)
> * The headers pushing code in videoencoder is broken, it makes the refs inside
> the list invalid. Also it should set discont on the first header buffer instead
> of the next non-header buffer (see discont handling code a few lines above) and
> probably also include the headers in priv->bytes

fixed

(In reply to comment #8)
> (In reply to comment #7)
> >  [...]
> > * The _event() vfuncs should require to chain up to the base class and the base
> > class should do all handling there instead of abusing the gboolean return
> > value. We changed that for all base classes in 0.11
> 
> For this keep the same behaviour as the audio base classes in 0.10 btw (not
> sure if they chain up or abuse the gboolean), and change it accordingly in
> 0.11.

So nothing to change in 0.10

(In reply to comment #7)
> gst_video_frame_set_coder_hook(frame, hook, destroy_notify). The problem here
> is with having the two fields in the struct and no obvious connection between
> them.

Done

> 
> Some new reviews :)
> 
> *
> http://cgit.freedesktop.org/~bilboed/gst-plugins-base/commit/?h=basevideo&id=5f76571bef4745340333f33f2985ea3f923fd383
> : Maybe rename GstVideoState to GstVideoCodecState too

Done

> * In general, use gst_object_ref/unref everywhere, otherwise twi will be
> unhappy. And myself too. When debugging refcount issues

Done

> * gst_video_decoder_set_output_state(), gst_video_encoder_set_output_state()
> should probably take the video-decoder streamlock, same probably elsewhere
> where the states are used. It's a recursive mutex btw. This might be necessary
> for things like gst-omx, where the sink and srcpad have separate streaming
> threads. Which is the only reason why these lock exists

 done

> *
> http://cgit.freedesktop.org/~bilboed/gst-plugins-base/commit/?h=basevideo&id=26a4ad1a95cc4c464d797cdf9a9adf2e4f0b1ddf
> : Remove the timestamp/offset stuff for headers completely. It's broken and
> useless

 Done

> * I think GST_FLOW_DROPPED is useless now, it was only used for EOS events and
> these can now be handled and not pushed by the base class correctly, right?


> * audio encoder/decoder base class has flush vfunc, video encoder/decoder has
> reset and finish vfuncs. Unify if possible
> * audio encoder base class has set_hard_resync(), video encoder has
> set_discont()  (is this the same at all?)

> * getters for setters in video encoder missing

done

> * Something like gst_audio_{de,en}coder_merge_tags() will be very useful to
> have

can be done later

> * No set_latency() in decoder


How do we want to handle this ? by frames, by min/max time ? By both ?
> * No timestamp tolerance in encoder/decoder (see audio base classes)
We haven't really needed this for any video decoder so far. I'd prefer to see
that later on.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.