After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 737401 - videobox: Use videoconvert APIs for format conversions
videobox: Use videoconvert APIs for format conversions
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 745207
Blocks:
 
 
Reported: 2014-09-26 05:23 UTC by RaviKiran
Modified: 2018-11-03 14:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
use videoconvert API for format conversion (95.04 KB, patch)
2014-10-28 11:48 UTC, RaviKiran
none Details | Review
screenshots of output (169.02 KB, application/gzip)
2014-11-27 09:17 UTC, RaviKiran
  Details
use video-converter APIs in videobox for format conversions (87.68 KB, patch)
2015-02-26 13:18 UTC, RaviKiran
none Details | Review
use videoconvert API for format conversion (88.93 KB, patch)
2015-03-05 06:13 UTC, RaviKiran
none Details | Review

Description RaviKiran 2014-09-26 05:23:11 UTC
(Following conversation in bug https://bugzilla.gnome.org/show_bug.cgi?id=733588)
Instead of converting image formats in videobox, use APIs provided by the videoconvert.
Comment 1 RaviKiran 2014-10-09 09:20:53 UTC
I'm working on this. Will submit the patch for review once completed.
Comment 2 RaviKiran 2014-10-28 11:48:10 UTC
Created attachment 289514 [details] [review]
use videoconvert API for format conversion

Patch attached for proposed changes.

Few observations:
Creating a converter requires height and width of both input and output frames to be equal.
Hence an intermediate frame to be created with height and width = that of output frame.
copy the contents from input frame to intermediate frame considering border area/crop area.
Then, call videoconvert API to convert intermediate frame to output frame of desired format.

This adds some overhead. For example, when I measured the time consumed,
conversion of AYUV to I420 of size 320x240 with border -10 each side, took ~988.6ms (against ~629ms earlier)

However, when input format = output format, this overhead is avoided.
AYUV to AYUV of size 320x240 with border -10 each side, took ~161.9ms (against ~288.5ms earlier)
I420 to I420 of size 320x240 with border -10 each side, took ~80.5ms (against ~839.8ms earlier)
[the saving here is mostly due to memcpy of lines instead of pixel-by-pixel copy]

As most of the videobox usage I came across were mostly about only adding borders or cropping, there will be no overhead in those cases with the new approach.

Feedback welcome.
Comment 3 Wim Taymans 2014-10-31 14:23:14 UTC
You can now use the cropping support in the converter directly. Could you try to update the patch? I think you can remove some more lines :)
Comment 4 RaviKiran 2014-11-26 10:32:20 UTC
Wim, using video-converter still crops the right side only (instead of left).
For example, I ran the pipeline 
gst-launch-1.0 videotestsrc ! "video/x-raw,format={RGB},height=480,width=640" ! videobox left=50 right=-50 ! videoconvert ! ximagesink
This should have cropped left region. But, it cropped the right side.

Part of the problem is because "unpack_RGB()" in video-format.c doesn't consider 'x' offset (actually, none of unpack functions do).
And, pack functions do not have 'x' offset parameter. 
https://bugzilla.gnome.org/show_bug.cgi?id=740228 is reported for this.

Once pack functions support 'x' parameter, I think we need to add 'x' to PACK_FRAME macro in video-converter.c
Comment 5 Wim Taymans 2014-11-26 10:58:52 UTC
(In reply to comment #4)
> Wim, using video-converter still crops the right side only (instead of left).
> For example, I ran the pipeline 
> gst-launch-1.0 videotestsrc ! "video/x-raw,format={RGB},height=480,width=640" !
> videobox left=50 right=-50 ! videoconvert ! ximagesink
> This should have cropped left region. But, it cropped the right side.

Works for me, what are you doing? Are you using this patch? I don't see you setting any offset in the options?

> 
> Part of the problem is because "unpack_RGB()" in video-format.c doesn't
> consider 'x' offset (actually, none of unpack functions do).
> And, pack functions do not have 'x' offset parameter. 
> https://bugzilla.gnome.org/show_bug.cgi?id=740228 is reported for this.

yes, we need to add support for unpacking from a different offset.

> 
> Once pack functions support 'x' parameter, I think we need to add 'x' to
> PACK_FRAME macro in video-converter.c

no, pack_frame does not need an x parameter, only unpack.
Comment 6 RaviKiran 2014-11-27 09:17:37 UTC
Created attachment 291621 [details]
screenshots of output

I used latest video-converter code. 
In videobox, while creating new video-converter, I used the configuration structure, with values for
GST_VIDEO_CONVERTER_OPT_SRC_X = src_x, GST_VIDEO_CONVERTER_OPT_SRC_Y = src_y,
GST_VIDEO_CONVERTER_OPT_DEST_X = dest_x, GST_VIDEO_CONVERTER_OPT_DEST_Y = dest_y,
GST_VIDEO_CONVERTER_OPT_SRC_WIDTH = GST_VIDEO_CONVERTER_OPT_DEST_WIDTH = crop_w
GST_VIDEO_CONVERTER_OPT_SRC_HEIGHT = GST_VIDEO_CONVERTER_OPT_DEST_HEIGHT = crop_h

(Let me know if it is incorrect)

src_x, src_y, dest_x, dest_y, crop_w, crop_h are calculated as in the attached patch.
For the above pipeline, the values are - src_x = 50, src_y = 0, dest_x = 0, dest_y = 0, crop_w = 590, crop_h = 480.
In this case it cropped the right side (attached pic VB_VC.png). Output of original videobox code for same pipeline - VB_O.png

Since unpack_rgb doesn't consider 'x', it unpacks from beginning where as it should have unpacked from pos 50.
When I added the 'x' offset for unpacking, it cropped the left side correctly. 
However during packing, it left a border at the beginning. (Pic - VB_VC1.png) 

Then I tried a hack - added 'x' offset to src while 'packing'. (Pic - VB_VC2.png)

Let me know if I got something wrong. Thanks.
Comment 7 RaviKiran 2015-02-26 13:18:14 UTC
Created attachment 297974 [details] [review]
use video-converter APIs in videobox for format conversions

All dependencies sorted. New patch attached. Please review.
Only issue is, sometimes ARGB -> BGRx conversion crashes. I have created bug for that - https://bugzilla.gnome.org/show_bug.cgi?id=745207
Comment 8 RaviKiran 2015-03-05 06:13:13 UTC
Created attachment 298602 [details] [review]
use videoconvert API for format conversion

Little correction to previous patch
Comment 9 Wim Taymans 2015-03-05 09:46:07 UTC
Seems to work for me. I noticed the old videobox can do subsample cropping for I420, video-converter can't do that yet.
Comment 10 RaviKiran 2015-03-06 03:57:43 UTC
Ok. Do you plan to do it in video-converter?
Comment 11 Wim Taymans 2015-03-06 11:18:36 UTC
(In reply to RaviKiran from comment #10)
> Ok. Do you plan to do it in video-converter?

For now it can only do cropping or add a border with a width/height that is a multiple of the subsampling.

The way it was done before is actually not correct anyway, it causes a chroma shift. If we want to implement this correctly we have to move the chroma with a half pixel when we crop the luma. Not sure it is worth it for now.
Comment 12 GStreamer system administrator 2018-11-03 14:54:20 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/129.