[Bug 738302] videorate: Should increase minimum buffer in allocation query

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #16 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> 2015-01-18 19:31:29 UTC ---
Sorry, completely miss-read the base class, and obviously we need to query
downstream first.

commit 2e264103e1acb1f78edd4b1e14ede23d4e5c49a8
Author: Nicolas Dufresne <nicolas.dufresne at collabora.co.uk>
Date:   Sun Jan 18 14:17:07 2015 -0500

    Revert "videorate: Implement allocation query"

    This reverts commit 3c04db4a307048db70ee1d08c1d62e26ad9569d8


(In reply to comment #15)
> Also you should IMHO hook into decide_allocation or propose_allocation, or at
> least let the base class' query function handle the allocation query *first*.
> You currently call it after you adjusted the values, but that means that the
> query does not contain the values from downstream yet.

It's a bit weird, but we only need to hook propose_allocation() here.

(In reply to comment #14)
> This change is not completely correct. You set the maximum number of buffers to
> MAX(min, max) but that's not correct IMHO. If the minimum becomes bigger than
> the maximum you will have to fail the allocation query instead.

I was using a approach of better try then be sorry, but indeed it will fail
late. The problem with failing the allocation query there is that the
information is then lost. It will also fail where it should have worked.
Upstream element can create their own pool (e.g. GstVideoPool). I think what
need to be done is drop the pool that don't fit. At the end, recheck if there
is at least one pool left, and if not, set one with pool = NULL. I'll propose a
patch and attach for review.

Note that strictly speaking, this bug is also solved through this patch. x264
was proposing no pool, min = max = latency.

commit acc9529a36ab7eef38539cce9ab7d11658df1fd4
Author: Nicolas Dufresne <nicolas.dufresne at collabora.co.uk>
Date:   Sun Jan 18 11:07:43 2015 -0500

    x264enc: Don't set an allocation maximum

    There is no reason x264enc should enforce a maximum allocation size.
    The maximum is normally set by buffer pool which cannot grow, but we
    don't offer a buffer pool. This would lead to stall when used with
    element that don't implement allocation query.

    Related to: https://bugzilla.gnome.org/show_bug.cgi?id=738302

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