[gstreamer-bugs] [Bug 152091] - aspect-ratio fixes

[bugzilla-daemon at bugzilla.gnome.org]

http://bugzilla.gnome.org/show_bug.cgi?id=152091
GStreamer | gst-plugins | Ver: HEAD CVS





------- Additional Comments From rbultje at ronald.bitfreak.net  2004-09-07 14:41 -------
Created an attachment (id=31388)
 --> (http://bugzilla.gnome.org/attachment.cgi?id=31388&action=view)
videoscale PAR fixes.

videoscale handles PAR solely in the _fixate() and _link() functions. It works
as follows: it waits for either pad to start negotiating, then it will try to
set a "fitting" caps on the other pad and set up scaling as that. There's a bug
in this logic, however. Firstly, the PAR and width/height variables of a
previous link attempt are being stored in the videoscale struct, and those are
being used for subsequent link attempts. This is wrong, because those values
might have changed. Secondly, some of those values are only being set *after*
negotiation of the other pad has succeeded, but are being used *to set up*
negotiation of this other pad. The order here is wrong.

What *should* happen is that, on a link attempt on some pad, we automagically
create a "fitting" caps for the other pad and try to set that. If it works, set
up and be happy, else moan and bail out. During each link attempt, we should
retrieve the current supported values for PAR and size from the otherpad before
trying to set it, and generate an optimal caps for the other pad from that in
the following order:
- passthru
- scale, but keeping PAR
- scale, changing PAR but keeping the PAR/width/height ratios intact
- scale and change PAR/width/height ratios
Only when all that is done, do we store the from/to_width/height in the
videoscale struct and use that for scaling in the _chain() function. The PAR
values can be discarded from here on because we only use them for linking: PAR
values are not needed for scaling itself.

One particular thing that grabbed my attention is that videoscale contains a
_fixate() function which calculates src size based on sink PAR (which must
already be there) and possible width/height/PAR values on the sink side. This
is wrong for two reasons:
- the link function can already do this, as my proposed patch does
- it's not bidirectional
Therefore, it should probably be removed. In my current patch, the code is
still there but is never actaully being used because either the sink PAR is
already set (and then we set a fixed caps on the src pad right in the _link()
function) or the sink PAR is not set yet.

Please comment.

------- You are receiving this mail because: -------
You are the assignee for the bug.
You are the QA contact for the bug.