GNOME Bugzilla – Bug 371108
videoscale sucks at basic mathematics when it comes to PAR
Last modified: 2010-05-13 09:17:30 UTC
videoscale's job is to scale video, preserving display aspect ratio, unless it is not able to at all. Given that 3 x 2 variables take part in the negotiation (w, h, par for two pads), and "preserving DAR" is an equation that links the six variables, p-a-r should be calculated on one side if all other variables are specified. For example, this pipeline: gst-launch -v videotestsrc ! video/x-raw-rgb,width=720,height=576,pixel-aspect-ratio=16/15 ! videoscale ! video/x-raw-rgb,width=768,height=576 ! fakesink silent=TRUE should negotiate a par of 1/1 on videoscale's src pad. instead it does this: [gst-head] [thomas@otto gst-plugins-bad]$ gst-launch -v videotestsrc ! video/x-raw-rgb,width=720,height=576,pixel-aspect-ratio=16/15 ! videoscale ! video/x-raw-rgb,width=768,height=576 ! fakesink silent=TRUE Setting pipeline to PAUSED ... /pipeline0/videotestsrc0.src: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 Pipeline is PREROLLING ... /pipeline0/capsfilter0.src: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /pipeline0/capsfilter0.sink: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /pipeline0/videoscale0.src: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /pipeline0/videoscale0.sink: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /pipeline0/capsfilter1.src: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /pipeline0/capsfilter1.sink: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /pipeline0/fakesink0.sink: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 Pipeline is PREROLLED ... Setting pipeline to PLAYING ... I'm pretty sure videoscale actually used to do this. Please help save mathematics from oblivion!
Created attachment 76767 [details] [review] Attempt at solving this This patch hasn't seen much testing, but seems to work sensibly. It gets rid of the bit where we set a preferred PAR, because we'll ensure that we preserve dar if possible, and that's really what we want. I think that code was a bit bogus; but would like some other opinions.
FWIW, I found that this patch doesn't actually work so well after all... needs work. I might get back to this some time.
Ping? any progress on this?
Created attachment 145128 [details] [review] Another attemp to solve it In gst_video_scale_transform_caps, when input caps have a fixated par, instead of setting a range for output par it sets the same par as input caps. For that reason it gst_video_scale_fixate_caps we couldn't calculate output par when output size is fixed but par is not. andoni@longo:~$ gst-launch-0.10 --gst-plugin-path=/usr/local/lib/gstreamer-0.10/ -v videotestsrc ! video/x-raw-rgb,width=720,height=576,pixel-aspect-ratio=16/15 ! videoscale ! video/x-raw-rgb,width=768,height=576 ! fakesink silent=TRUE Estableciendo el conducto a PAUSA … /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0.GstPad:src: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 El conducto está PREPARÁNDOSE … /GstPipeline:pipeline0/GstCapsFilter:capsfilter0.GstPad:src: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /GstPipeline:pipeline0/GstCapsFilter:capsfilter0.GstPad:sink: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /GstPipeline:pipeline0/GstVideoScale:videoscale0.GstPad:src: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)1/1 /GstPipeline:pipeline0/GstVideoScale:videoscale0.GstPad:sink: caps = video/x-raw-rgb, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)720, height=(int)576, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)16/15 /GstPipeline:pipeline0/GstCapsFilter:capsfilter1.GstPad:src: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)1/1 /GstPipeline:pipeline0/GstCapsFilter:capsfilter1.GstPad:sink: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)1/1 /GstPipeline:pipeline0/GstFakeSink:fakesink0.GstPad:sink: caps = video/x-raw-rgb, width=(int)768, height=(int)576, bpp=(int)32, endianness=(int)4321, depth=(int)24, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, framerate=(fraction)30/1, pixel-aspect-ratio=(fraction)1/1 El conducto está PREPARADO … Estableciendo el conducto a REPRODUCIENDO … New clock: GstSystemClock
Created attachment 145129 [details] [review] Fix small mistake in previous path
Created attachment 145131 [details] [review] Added debug output and check if out pat is already fixated
Another case it should succesfully handle is the following: * input caps : width=1920,height=2560,pixel-aspect-ratio=1/1 * output caps : width=[1, 2048],height=[1, 2048],pixel-aspect-ratio=1/1 Currently videoscale will end up outputting width=1920,height=2048,pixel-aspect-ratio=1/1 ... which is breaking the DAR (no longer 3/4) It should instead output : width=1536,height=2048,pixel-aspect-ratio=1/1
Created attachment 160880 [details] [review] Patch for PAR when all the values are fixed except the output PAR Updated previous patch in git format
Created attachment 160881 [details] [review] Par for PAR when the out caps are not fixed but input caps are not in the range of out caps All the test passed successfully after applying both patches and the fixed the 2 bug related with PAR. I'm also writing new unit tests to check the PAR/DAR is always respected by videoscale
(In reply to comment #9) > Created an attachment (id=160881) [details] [review] > Par for PAR when the out caps are not fixed but input caps are not in the range > of out caps > > All the test passed successfully after applying both patches and the fixed the > 2 bug related with PAR. That's an improvement but not giving the perfect resolution in all cases. I'm working on this already, from a similar starting point as your latest patch. > I'm also writing new unit tests to check the PAR/DAR is always respected by > videoscale ...which is what I've done locally already. So if you didn't start already, don't do it :)
Andoni, check the status of the bug next time. When it's marked ASSIGNED, that means that somebody is actively working on it. Not that we don't want your patches, just that you would have saved some time :)
Uups. I'll check the status field next time :)
BTW, the last patch I send is wrong. Anyway the problem comes when the input caps are not in the same range as the output caps. You first compute the output width and height and afterwards fixate it to the nearest value. If one of them is out of range it will be clipped screwing up DAR.
commit 303566654e254834c7ab21be790f31d4e3f6f8d5 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon May 10 17:09:28 2010 +0200 videoscale: Add a unit test for checking if the negotiation works as expected commit b3808a57d4903ee4be12d8b3233ec57b025158db Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon May 10 17:09:28 2010 +0200 videoscale: Try harder to keep the DAR if possible Fixes bug #371108. commit 75a2e14e06bcba263f304c0e4b036f9222f77fd3 Author: Andoni Morales <ylatuya@gmail.com> Date: Mon May 10 13:01:44 2010 +0200 videoscale: Try to keep DAR when scaling Fixes bug #371108.