GNOME Bugzilla – Bug 636106
autocolorspace: new plugin for auto space convertor selection
Last modified: 2010-12-16 09:15:13 UTC
add a new element named autocolorspace, based on autoconvert, to perform the selection of the best color space converter This is done under Linaro project and specification: https://wiki.linaro.org/WorkingGroups/Middleware/Multimedia/Specs/1105/AutopluggableColorspaceConverter
Created attachment 175518 [details] [review] 1st autocolorspace implementation proposal
I also had that idea some time ago and made a wiki page for it, but there was not much interest for it. I also had an implementation but can't find it back anymore. http://gstreamer.freedesktop.org/wiki/ColorSpaceBin
yes it is the same idea does your implementation was also based on autoconvert ?
This is a good idea I have seen this issue in multiple HW. Until now we have been solving this by adding support under the decoder element for multiple color formats, but this is a more generic solution
Created attachment 175553 [details] [review] autocolorspace with fixed klass detection followed bilboed comment's: rewrite klass detection method
What exactly was the reason why you chose to use autoconvert as base class here? IIRC there are still some quirks in autoconvert because of it's very generic behaviour... Also you should probably make autocolorspace a GstBin with an autoconvert child instead of subclassing autoconvert. This way you would hide all the autoconvert GObject properties like "factories", which you really don't want the user the override :)
I have chose to use autoconvert because it seem to do all the boring staff around selection and and pad linking. If autoconvert as to much quirks and it not usable yet I could rewrite autocolorspace from GstBin and use function like decodebin try_to_link_1 function to do the same. is it a better solution?
Oh, and another thing. You should refresh the factory list when going to READY, this way automatic codec installation and similar things would work. I guess using autoconvert here is fine if it works... and it would give a reason to improve any problems with autoconvert :) But still, you should make autocolorspace a GstBin with a single autoconvert child
Created attachment 176389 [details] [review] autocolorspace a GstBin with an autoconvert child I make autocolorspace a GstBin with an autoconvert child and only set factories properties when state change to READY. I a perform some tests if rgb2bayer is present gst-launch videotestsrc num-buffers=2 ! "video/x-raw-rgb,width=100,height=100,framerate=10/1" ! autocolorspace ! "video/x-raw-bayer,width=100,height=100,format=bggr,framerate=10/1" ! fakesink -v if bayer2rgb is present gst-launch videotestsrc num-buffers=2 ! "video/x-raw-bayer,width=100,height=100,format=bggr,framerate=10/1" ! autocolorspace ! "video/x-raw-rgb,width=100,height=100,framerate=10/1" ! fakesink -v test with ffmpegcolorspace gst-launch videotestsrc num-buffers=2 ! "video/x-raw-rgb,bpp=32,width=100,height=100,framerate=10/1" ! autocolorspace ! "video/x-raw-rgb,bpp=16,width=100,height=100,framerate=10/1" ! fakesink -v
Created attachment 176394 [details] [review] same + global factories list same patch than before + global factories list protected by mutex
Created attachment 176402 [details] [review] test suite for autocolorspace
I've committed both patches now but please attach patches in git format-patch format in the future :) commit 8b0c2db425563f3b773825138fcf13c03cca120a Author: Benjamin Gaignard <benjamin.gaignard@stericsson.com> Date: Wed Dec 15 21:19:55 2010 +0100 autocolorspace: Add unit test commit 27ac6c3e473bcf5472cfb5c7817c122349d44531 Author: Benjamin Gaignard <benjamin.gaignard@stericsson.com> Date: Wed Dec 15 21:14:38 2010 +0100 autocolorspace: Add autoconvert based video format convert element Fixes bug #636106.
If I read the code correctly, it will match all of these plugins: ssim: Class: Filter/Converter/Video alphacolor: Class: Filter/Converter/Video rgb2bayer: Class: Filter/Converter/Video bayer2rgb: Class: Filter/Converter/Video ffdeinterlace: Class: Filter/Converter/Video ffmpegcolorspace: Class: Filter/Converter/Video ffvideoscale: Class: Filter/Converter/Video Is that really what you want ? That said, maybe the classes are wrong: - Possibly ffvideoscale should be "Filter/Effect/Video" like videoscale. - ssim clearly isn't a converter. I guess the rest are fine.
you are right Olivier, that what I wanted, and yes some class need to be fixed. It is already done for ssim (https://bugzilla.gnome.org/show_bug.cgi?id=636109)
Arguably videoscale should be Filter/Converter/Video too but ffdeinterlace (and ssim) shouldn't Also Tim said that this element should better be called autovideoconvert, it doesn't only do colorspace conversion. I'll rename it later
Ok, it's renamed now, ffdeinterlace is no Converter anymore and the two scalers we have are Filter/Converter/Video/Scaler now