G-API gst source gray support by dbudniko · Pull Request #21560 · opencv/opencv

dbudniko · 2022-02-03T13:34:45Z

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

force_builders=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.4.1:20.04
build_image:Custom Win=openvino-2021.4.1
build_image:Custom Mac=openvino-2021.4.1

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

…rmat_gray

…/opencv into dbudniko/gapi_media_format_gray

modules/gapi/src/streaming/gstreamer/gstreamer_media_adapter.cpp

OrestChura · 2022-02-04T13:41:23Z

modules/gapi/src/streaming/gstreamer/gstreamer_media_adapter.cpp

-            GAPI_Assert(GST_VIDEO_INFO_FORMAT(m_videoInfo.get()) == GST_VIDEO_FORMAT_NV12);
+            //GAPI_Assert(GST_VIDEO_INFO_N_PLANES(m_videoInfo.get()) == 2);
+            GAPI_Assert(GST_VIDEO_INFO_FORMAT(m_videoInfo.get()) == GST_VIDEO_FORMAT_NV12 ||
+                        GST_VIDEO_INFO_FORMAT(m_videoInfo.get()) == GST_VIDEO_FORMAT_GRAY8);


Maybe, these asserts above should be combined, like

GAPI_Assert((GST_VIDEO_INFO_N_PLANES(m_videoInfo.get()) == 2 && GST_VIDEO_INFO_FORMAT(m_videoInfo.get()) == GST_VIDEO_FORMAT_NV12) || (GST_VIDEO_INFO_N_PLANES(m_videoInfo.get()) == 1 && GST_VIDEO_INFO_FORMAT(m_videoInfo.get()) == GST_VIDEO_FORMAT_GRAY8));

But perhaps it looks too heavy

modules/gapi/src/streaming/gstreamer/gstreamer_media_adapter.cpp

modules/gapi/src/streaming/gstreamer/gstreamersource.cpp

modules/gapi/test/streaming/gapi_gstreamersource_tests.cpp

dmatveev

68 commits is way too large, recommend to squash it before merge

modules/gapi/src/streaming/gstreamer/gstreamer_media_adapter.cpp

dmatveev · 2022-02-04T17:28:53Z

modules/gapi/src/streaming/gstreamer/gstreamersource.cpp

+                        break;
+                    }
+                    case GST_VIDEO_FORMAT_GRAY8: {
+                        m_mediaFrameMeta = GFrameDesc{ cv::MediaFormat::GRAY, cv::Size(width, height) };


so is it GRAY or GRAY8? I believe I've seen GRAY8 somewhere in the doxygen comments (and I believe it is quite legit)

There are different Gst gray formats. GRAY8 GRAY16. MediaFormat GRAY assumes 8-bit integer numbers. Should we align our naming approach with Gst. I have some doubts that 16 bit will be asked by someone.

I have some doubts that 16 bit will be asked by someone.

It seem to be useful for some scenarios, see #18694. Furthermore, there are two variants of this format: GRAY16_LE and GRAY16_BE

dmatveev · 2022-02-04T17:29:37Z

modules/gapi/src/streaming/gstreamer/gstreamersource_priv.hpp

    bool m_isPipelinePlaying = false;

    int64_t m_frameId = 0L;
+    size_t m_type = 0;


What is this?

Comment added.

dbudniko · 2022-02-07T12:09:56Z

68 commits is probably result of master merge with media format gray PR merged. I assume that these commits will be automatically squashed during PR final merge. Please correct me if I'm wrong. What is the right way to squash such commits in OpenCV GitHub?

OrestChura

Thanks!

dbudniko · 2022-02-08T09:44:08Z

@alalek could you please merge this PR?

alalek · 2022-02-08T09:55:37Z

modules/gapi/test/streaming/gapi_gstreamersource_tests.cpp

                        Combine(Values("videotestsrc pattern=colors num-buffers=10 ! "
                                       "videorate ! videoscale ! "
-                                       "video/x-raw,width=640,height=420,framerate=3/1 ! "
+                                       "video/x-raw,format=NV12,width=640,height=420,framerate=3/1 ! "
+                                       "appsink",
+                                       "videotestsrc pattern=colors num-buffers=10 ! "
+                                       "videorate ! videoscale ! "
+                                       "video/x-raw,format=GRAY8,width=640,height=420,framerate=3/1 ! "
                                       "appsink"),


It makes sense to add extra indentation for concatenated strings to improve readability of the code.

…mat_gray_plus_gst_source G-API gst source gray support

Dmitry Budnikov added 30 commits January 25, 2022 12:29

gray draft

e0cb364

Merge remote-tracking branch 'origin/4.x' into dbudniko/gapi_media_fo…

a7e0ea8

…rmat_gray

remove uv_plane from gray code path

a5956e6

trying to address comments from Asya (accessors are modified)

198e6e1

try to push one more time

52adfdc

address more comments from Asya and Sergey

987ce91

add gray to streaming tests

bd796c8

some gray source corrections and copy gray frame

6795ecf

try to fix copy frame gray test

10bc6fd

try to add reshape gray test

338473e

fix test source

76a6735

disable reshape gray test

3fc50fa

debug prints

1886642

Merge branch 'dbudniko/gapi_media_format_gray' of github.com:dbudniko…

7a308cb

…/opencv into dbudniko/gapi_media_format_gray

try to convert BGR to gray

e7e98ec

roll back

10d6b76

try to fix gray reshape test

8e83d55

clean up

add328f

try bgr to gray again

7983d37

try to add media frame tests

7f3fdb7

size media frame gray test

da64f69

more comments are addressed

8446626

rmat on mat adapter

f74f205

more streaming tests

1fae2d0

try to add more test instances

f8a0dca

try to fix accuracy

155a2cd

remove source type gray

1884dd0

streaming tests fix - source type gray

d89a8d4

fix streaming test logic

a272fe1

try to fix test

e025398

Dmitry Budnikov added 11 commits February 3, 2022 18:58

clean up and try to use default in switches

41212cb

more clean-ups

48d4ce2

NV12 check

8306134

try to fix build and clean up

487f9b0

compile args fix

574ac9a

build fixed

561cd42

fix logic

aa2d906

try to fix computation

c1e191a

build ok tests ok

d7c8cce

refactor tests

9b2a150

clean up

ab338ef

OrestChura reviewed Feb 4, 2022

View reviewed changes

assert fix

29e8acb

OrestChura reviewed Feb 4, 2022

View reviewed changes

modules/gapi/test/streaming/gapi_gstreamersource_tests.cpp Show resolved Hide resolved

modules/gapi/test/streaming/gapi_gstreamersource_tests.cpp Show resolved Hide resolved

dmatveev self-assigned this Feb 4, 2022

dmatveev added this to the 4.6.0 milestone Feb 4, 2022

dmatveev reviewed Feb 4, 2022

View reviewed changes

Dmitry Budnikov added 3 commits February 7, 2022 16:04

more comments are addressed

be281dd

Address comments from Orest

7e310a6

roll back

fe2927d

OrestChura approved these changes Feb 7, 2022

View reviewed changes

dbudniko unassigned dmatveev Feb 8, 2022

alalek assigned OrestChura Feb 8, 2022

alalek reviewed Feb 8, 2022

View reviewed changes

alalek merged commit 3eeec4f into opencv:4.x Feb 8, 2022

TolyaTalamanov pushed a commit to TolyaTalamanov/opencv that referenced this pull request Feb 18, 2022

Merge pull request opencv#21560 from dbudniko:dbudniko/gapi_media_for…

1db0cbe

…mat_gray_plus_gst_source G-API gst source gray support

alalek mentioned this pull request Feb 22, 2022

(5.x) Merge 4.x #21651

Merged

a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023

Merge pull request opencv#21560 from dbudniko:dbudniko/gapi_media_for…

97865db

…mat_gray_plus_gst_source G-API gst source gray support

Uh oh!

Conversation

dbudniko commented Feb 3, 2022

Pull Request Readiness Checklist

Uh oh!

Uh oh!

OrestChura Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmatveev Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

dbudniko Feb 7, 2022

Choose a reason for hiding this comment

Uh oh!

mshabunin Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

dmatveev Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

dbudniko Feb 7, 2022

Choose a reason for hiding this comment

Uh oh!

dbudniko commented Feb 7, 2022

Uh oh!

OrestChura left a comment

Choose a reason for hiding this comment

Uh oh!

dbudniko commented Feb 8, 2022

Uh oh!

alalek Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants