[Bug 762543] xvimagesink: wrong pool selection when rendering buffers having GstVideoCropMeta attached

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=762543

--- Comment #17 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Summary from IRC (this should become a duplicate of the other crop meta bug):

<ndufresne> slomo: using crop meta won't work for the scenario where downstream
pool is not used, this would require all video sink that supports crop meta to
interpret propose_allocation as being a set_padded_caps() or something
<slomo> ndufresne: why would they have to interprete and allocation query as
some kind of set_caps()?
<ndufresne> slomo: the problem is that the sink need to copy that frame,
because it's wrong memory type, for that it uses GstVideoFrame, and the only
video information we have is that caps
<ndufresne> Because this is how everything is supposed to work
<ndufresne> but now, the incoming buffers have a width/height in it's
GstVideoMeta that matchs the caps passed in proposed_allocation
<ndufresne> so frame_map() fails, no video sink is expecting that atm
<slomo> i see
<slomo> that sounds solveable though?
<slomo> everything supporting crop meta should handle that
<slomo> videoinfo from caps to frame_map() with display size, buffer contains
GstVideoInfo with memory size... and then things should do something sensible
;)
<ndufresne> slomo: we could, it makes everything more complex then it should
imho
<ndufresne> I believe the original idea, was that sink don't need to extract
the VideoMeta and make a decision
<ndufresne> That's why video_frame_map() was overriding width/height silently
before
<ndufresne> slomo: do you remember exactly why you added this restriction ?
<ndufresne> cause, an element that expect CropMeta, would expect larger
width/height in the VideoInfo structure after mapping the VideoFrame
<slomo> ndufresne: yes, i didn't consider crop meta ;)
<slomo> ndufresne: i think if crop meta is on the buffer, gst_video_frame_map()
must check different restrictions. that the meta+crop resolution is equal to
the one from the video info
<ndufresne> I think what I'd like, is to be able to say that we can map a frame
without having to extract the VideoMeta ourself, this is the main feature of
using VideoFrame after all
<slomo> ndufresne: i added this restriction because there were elements that
forgot to update their caps/video info and then just crashed in interesting
ways
<ndufresne> slomo: that is a good option
<ndufresne> Now, it's not enought
<slomo> why not enough?
<ndufresne> yes, we need to improve video_frame_copy() too
<slomo> oh, sure
<ndufresne> it does not support odd crops
<slomo> but frame_map() should be happy with that so far
<ndufresne> so my conclusion, correct me if wrong, is that what xvimagesink
(and all others) do, blindly so frame_copy() is correct, and we need to check
the presence of a crop meta in frame_map
<slomo> yes
<slomo> and adjust the sanity check for width/height accordingly
<ndufresne> sounds to be like something I can work on, as a first step, I'll
try and also detect odd crops (that could cause UV shift) and warn
<slomo> can you put our discussion in the bug for future reference?
<ndufresne> yes
<slomo> isn't there also another bug about the general crop meta problem
somewhere?
<ndufresne> yes, there is I believe, need to find it, with this conclusion,
it's now a duplicate
<ndufresne> slomo: if I implement CropMeta in videocrop, what would the
downstream caps be btw ?
<slomo> the same as they are now
<slomo> the caps event has the caps it would have now too
<ndufresne> ... ! video/x-raw,width=100,height=100 ! videocrop top=10 bottom=10
left=10 right=10 ! video/x-raw,...
<slomo> only difference is the allocation query, it would have the sinkpad caps
<slomo> 80x80 in your example
<ndufresne> ok, 80x80, with a crop meta on buffers, that we agree then, great
<ndufresne> for a long time, I believed that crop meta was something we could
add to a buffer, without affecting the caps
<slomo> so that in frame_map() you can calculate 80+10+10==100 and fail
otherwise ;)
<slomo> yes the documentation about this is not existing
<ndufresne> well, the crop meta is using a rectangle (x,y,width,height), but we
need to check that it's within the original size of course

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