GNOME Bugzilla – Bug 626518
[imagefreeze] better caps negotiation
Last modified: 2010-09-04 13:17:12 UTC
Created attachment 167490 [details] GES debug log (ges uses fast pad linking) It seems that imagefreeze sometimes fails to negotiate downstream caps properly without an ffmpegcolorspace after the freeze this pipeline works for me: gst-launch-0.10 uridecodebin uri=<jpeg file> ! videoscale ! ffmpegcolorspace ! imagefreeze ! xvimagesink the situation seems to arise when imagefreeze is used inside of a gnlsource inside a composition. I'm attaching a python script which exemplifies the problem and two debug logs. The following is an excerpt from an irc conversation: 10:24 < slomo> is there a ffmpegcolorspace between imagefreeze and the sink? 10:25 < emdash> that I don't know 10:25 < emdash> i didn't add one 10:25 < emdash> so my guess is not 10:26 -!- muelli [~muelli@port-697.pppoe.wtnet.de] has joined #gstreamer 10:26 < slomo> emdash: that's the problem then probably 10:26 < slomo> maybe we need better negotiation in imagefreeze 10:27 < slomo> please file a bug, i'll look at it later 10:27 < emdash> yeah, i'm heading home too 10:28 < slomo> ok :) 10:29 < emdash> maybe there's a reason we don't have a colorspace before the sink 10:29 -!- muellisoft [~muelli@port-11268.pppoe.wtnet.de] has quit [Ping timeout: 276 seconds] 10:29 < emdash> i'm looking at the code for ges-timeline-pipeline and it doesn't appear to stick anything 10:29 < slomo> emdash: it shouldn't be needed
Created attachment 167491 [details] python script which exemplifies the bug running the script on .png files seems to work as expected for me. running the script on .jpg files (which seem to decode to yuv) does not.
Created attachment 167492 [details] debug log of pythong script failing on a jpeg file
Created attachment 167495 [details] debug log of python script working fine on .png file
Created attachment 167539 [details] [review] imagefreeze: Passthrough buffer allocations
(In reply to comment #4) > Created an attachment (id=167539) [details] [review] > imagefreeze: Passthrough buffer allocations So the problem here is quite complex ;) First of all, your example only fails if the picture is larger than what XV can handle. The problem is, that gnlsource and gnlcomposition are adding their srcpads only after a pad block (e.g. after a buffer has arrived). But that's too late to do any renegotiation, the format is already fixed and everything... and makes the linking fail. If there's a ffmpegcolorspace and/or videoscale after imagefreeze, the pad block will already be caused by a bufferalloc. In that case downstream still has the chance (after everything is linked) to change the format and everything will work. So the first step was, to add bufferalloc passthrough to imagefreeze. This will only work without a ffmpegcsp/vscale after imagefreeze if the decoder uses bufferalloc though, so there should always be a second ffmpegcsp/vscale after imagefreeze. But it would work in passthrough mode if the decoder did bufferalloc. For most cases you then convert the frame once before imagefreeze and everything is fine. The remaining problem now is, that there are many pad blocks and the pad block from gnlcomposition on the srcpad of gnlsource is causing a flushing seek. This flushing seek causes the blocked bufferalloc to return wrong-state and everything fails again.
And I think the flushing problem is something that has to be fixed somewhere in gnonlin. For normal pipelines this is no problem because the element that handles the seek and is driving the pipeline will *pull* from upstream (or is the source) but in the case of imagefreeze (where it gets things pushed from upstream and the seek is not forwarded) this simply doesn't work because it disrupts the buffer allocation chain.
Ok, I have a solution in imagefreeze. Now this only needs some changes elsewhere but it can be fixed ;)
Created attachment 167626 [details] [review] xvimagesink: It's not a bad thing if the preferred video format needs less bytes per frame
Created attachment 167627 [details] [review] videoscale: Only set the PAR if the caps already had a PAR Otherwise we're producing different caps and basetransform thinks that it can't passthrough buffer allocations, etc. In 0.11 all video caps really should have the PAR set...
Created attachment 167628 [details] [review] videoscale: Add some debug output to the videoscale negotiation test
Created attachment 167629 [details] [review] xvimagesink: Suggest caps with different width/height if bufferalloc is called with impossible width/height
Created attachment 167630 [details] [review] imagefreeze: Passthrough buffer allocations
Created attachment 167631 [details] [review] imagefreeze: Return GST_FLOW_UNEXPECTED when getting a second buffer This prevents upstream from pushing many useless buffers and makes it go into EOS state.
Created attachment 167632 [details] [review] imagefreeze: Retry bufferalloc if it was aborted with WRONG_STATE because of a flushing seek
With all these patches (base patches are pushed already) this works in most cases. A pipeline like src ! conv ! scale ! freeze ! sink will always work now *if* the source uses bufferalloc and uses a format that is supported by ffmpegcolorspace *and* videoscale. In all other cases this will still abort with not-negotiated. If you only care about png and jpeg this will always work. If you add converters after freeze, the conversions will take place after freeze because you can only know the sink caps after the bufferalloc has advanced to the last element inside the gnlcomposition and the sink has to suggest new caps in bufferalloc. The elements most downstream will then try to establish the conversion and the conversion will take place after imagefreeze. The other solution here would be, to add a capsfilter with the sink's caps right before imagefreeze... but that only works of course if you can know the sink caps in advance. So for GES there are three possible solutions now: a) if you know the sink caps in advance add a capsfilter b) if you only care about jpeg/png only use converters before imagefreeze c) otherwise add converters before and after imagefreeze and live with the conversions taking place after imagefreeze most of the time...
How hard would it be to write a unit test that checks some of this?
Looks good to me. A unit test would be nice indeed.
I'll make tests before pushing this... one for the bufferalloc, one for the renegotiation and one for the UNEXPECTED
Pushed now