[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 #6 from Edward Hervey <bilboed at gmail.com> 2012-03-29 13:55:32 UTC ---
(In reply to comment #5)
> Some more comments here:
> 
> *
> http://cgit.freedesktop.org/~bilboed/gst-plugins-base/commit/?h=basevideo&id=bb8a8c3b0491f0718c98d925c17dc47958cca4a9:
> ** leaking previously set input state

Fixed

> *
> http://cgit.freedesktop.org/~bilboed/gst-plugins-base/commit/?h=basevideo&id=54cb2a569b4ed629bfa60f8d978699d5c30cacd1:
> ** the comment at the top talks about channels/rate

Fixed 

> *
> http://cgit.freedesktop.org/~bilboed/gst-plugins-base/commit/?h=basevideo&id=10fdd482ddfb0094e49ddd97477e30ff802e8274:
> ** why are there two functions for the caps? can't this be one and detection if
> this is raw (video/x-raw) or not is inside the function?
> ** and the parameters to the compare function can be const
> *

Fixed, added a new GstVideoFormat for codecs instead and only have one method

> http://cgit.freedesktop.org/~bilboed/gst-plugins-base/commit/?h=basevideo&id=a3aa0b63b4f3aa8a8c66c3d4c9fb4b2d537f3419:
> ** can we get rid of the clean_* fields too?
> * Two states are missing in the encoder currently

Fixed

> 
> And from previous review, just so it doesn't get lost:
> * sink_event/src_event vfuncs missing

Fixed

> * two public segments (in and output segment) or make part of the states

Fixed

> * gst_video_decoder_get_byte_time() is a really weird name :)

renamed it to _estimate_rate()

> * GstVideoState/GstVideoFrame needs padding and probably still contains some
> unused things. But I know that you planned to work on this next

Added padding.

Was wondering if we couldn't have some private entry for cleaner usage ...

> * Check the ownership transfer of the frame parameter of _finish_frame(),
> _drop_frame() and ::handle_frame(). Should be consistent with the ones in the
> audio base classes and IIRC they are not

Done

> * gst_video_decoder_set_sync_point() and other frame-related things should
> probably operate on the frames  nstead of the decoder. Feels weird to do it on
> the decoder instead and also could be problematic if there's internal queueing
> of more than a single frame inside the decoder

Removed (callers can just set frame->is_sync_point=TRUE instead)

> * The coder_hook/destroy_notify is probably hard to use for bindings

What would a better option be ?


Pushed all changed to my repos (base and bad)

-- 
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.