GNOME Bugzilla – Bug 158650
[PATCH] [videocrop] video crop is completely buggered
Last modified: 2004-12-22 21:47:04 UTC
I'm trying to make the videocrop element work as part of a DVD-rip-and-encode pipeline, but so far even the most simple gst-launch pipelines fail. a) gst-launch-0.8 videotestsrc ! videocrop left=8 ! xvimagesink looks like there's a row-stride issue - the line offsets are moved by a constant with each line => garbled picture b) gst-launch-0.8 videotestsrc ! videocrop top=8 ! xvimagesink looks like the top 8 rows have been shifted to the bottom of the picture? c) gst-launch-0.8 dvdreadsrc location=/dev/sr1 title=4 ! dvddemux ! mpeg2dec ! videocrop ! ffmpegcolorspace ! xvimagesink picture is completely unrecognizable (but it's still not white snow at least); size of window seems to be wrong as well (ca. 384x288 instead of the full size that you get without the videocrop element). d) gst-launch-0.8 dvdreadsrc location=/dev/sr1 title=4 ! dvddemux ! mpeg2dec ! videocrop left=4 right=4 ! ffmpegcolorspace ! xvimagesink similar to (c) e) gst-launch-0.8 dvdreadsrc location=/dev/sr1 title=4 ! dvddemux ! mpeg2dec ! ffmpegcolorspace ! videocrop ! videoscale ! video/x-raw-yuv,width=384,height=288 ! xvimagesink => endless chain of assertion failures: ** (process:2053): CRITICAL **: file gstvideoscale.c: line 547 (gst_videoscale_chain): assertion `size == videoscale->from_buf_size' failed f) gst-launch-0.8 dvdreadsrc location=/dev/sr1 title=4 ! dvddemux ! mpeg2dec ! ffmpegcolorspace ! videocrop left=4 right=4 ! videoscale ! video/x-raw-yuv,width=384,height=288 ! xvimagesink same as (e). Marking this as a blocker. Feel free to change this, but having a completely broken cropping element in gst-plugins is a bit bad IMHO. Cheers -Tim
Removing blocker flag; looking at the code, I don't think it ever really worked in the 0.8 series, so nobody seems to be using it or missing it. And while it is something I could really need for my DVD ripping application, I don't think it warrants blocker status after all. Cheers -Tim
Check out videobox instead, which does seem to work. If videocrop doesn't work at all, as it appears, then clearly no-one is using it, and we should delete it entirely.
Thanks, I did try videobox, but it was also not really working all too well (even though it seems less broken than videocrop). I don't think videocrop should be deleted. It should be fixed. It's a simple element and it has a brilliant name (I would never have guessed that 'videobox' actually crops stuff). And it does work for pictures with well-formed dimensions (e.g. width and height = multiple of 8), just not very well for pictures with odd dimensions. Sorry for not making this clearer. The GST_VIDEO_I420_FOO macros should probably be fixed to match those that are used elsewhere (note the round-up of the strides), and GST_VIDEO_I420_SIZE should be used when allocating the new buffer etc. Might whip up a patch over the weekend. Cheers -Tim
Actually, I take that back: Videocrop is completely buggered for all widths/heights. It doesn't even seem to set its source caps to the new smaller picture size - no wonder the image sinks show rubbish. Patch coming up. Cheers -Tim
Created attachment 34740 [details] [review] proposed patch Patch that makes all of the above pipelines work for me. Most notably it: * updates the GST_VIDEO_I420_FOO() macros to those used elsewhere (note the roundups, and the size macro) * fixes videocrop to allocate a properly-sized buffer for the output (used to be too small at times because it didn't take into account rowstride padding) * fixes videocrop to use correct rowstrides when cropping and copying * sets source caps to original_video_size - cropping (not sure I've done it the right way, but it seems to work and even survives ximagesink window resizing) Cheers -Tim
It still has issues with caps proxying, I don't think it works in two ways: $ ../../../gstreamer/tools/gst-launch videotestsrc ! ffmpegcolorspace ! videocrop top=8 bottom=48 ! xvimagesink RUNNING pipeline ... ERROR: from element /pipeline0/videocrop0: Internal GStreamer error: negotiation problem. File a bug. ERROR (0x82ec910 - 306443:43:49.261826000) scheduler(10179) gstoptimalscheduler.c(2638):gst_opt_scheduler_iterate:<GstOptScheduler@0x8493a38> in error state Execution ended after 1 iterations (sum 5953000 ns, average 5953000 ns, min 5953000 ns, max 5953000 ns). Removing the ffmpegcolorspace element makes it work. It fixes other sides. I can apply this, but it won't fix the bug, negotiation is still buggered...
Created attachment 34947 [details] [review] new patch Here's a new patch: * fixes rowstride issues when cropping * fixes caps negotiation * uses gst_pad_alloc_buffer() instead of gst_buffer_new_alloc() * remove EVENT_AWARE flag and event handling code All of the above tests pass now for me. Cheers -Tim
Looks good. Applied. Go get a CVS account, you! :).