feat: SetFps v4l2 + ffmpeg command by ChristianDarr-personal · Pull Request #265 · edgexfoundry/device-usb-camera

ChristianDarr-personal · 2023-07-13T17:20:36Z

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
I am not introducing a new dependency (add notes below if you are)
I have added unit tests for the new feature or bug fix (if not, why?)
I have fully tested (add details below) this the new feature or bug fix (if not, why?)
I have opened a PR for the related docs change (if not, why?)

Testing Instructions

Note, any commands executed need to include the query parameter PathIndex with the value set to the number representing the video path stream intended. For example: http://localhost:59982/api/v3/url?PathIndex=0. Also it is recommended to use the updated postman collection.

Execute the FrameRateFormat api to see supported fps values. This shows all fps values for each resolution for each type of encoding. This can be useful for determining the values to set the camera fps.
Execute the DataFormat api to see the supported fps values for the current video format and encoding.
Execute the SetFrameRate api and see no errors.The fps is represented in a time per frame format, meaning the denominator divided by the numerator yields the fps value. I did this to maintain consistency with the internal data structures. You can leave the numerator out completely, and the denominator will represent the fps value.
Execute the GetFrameRate api and see that the fps match.
Test the video stream and/or raw device stream to see if the changes apply. Certain video players seem to reset the fps value themselves, so it may not be viable to see the fps value of the /dev/video stream reflected in them.

New Dependency Instructions (If applicable)

Signed-off-by: Darr, Christian <christian.darr@intel.com>

codecov-commenter · 2023-07-14T22:03:09Z

Codecov Report

Merging #265 (defad8f) into main (7adbeca) will decrease coverage by 0.74%.
The diff coverage is 0.00%.

❗ Current head defad8f differs from pull request most recent head db557b0. Consider uploading reports for the commit db557b0 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff            @@
##            main    #265      +/-   ##
========================================
- Coverage   4.95%   4.21%   -0.74%     
========================================
  Files          8       8              
  Lines        908    1067     +159     
========================================
  Hits          45      45              
- Misses       863    1022     +159

Files Changed	Coverage Δ
internal/driver/device.go	`6.15% <0.00%> (-5.28%)`	⬇️
internal/driver/driver.go	`3.79% <0.00%> (-0.52%)`	⬇️
internal/driver/ffmpeg.go	`16.50% <0.00%> (ø)`
internal/driver/metadata.go	`0.00% <0.00%> (ø)`

vyshali-chitikeshi · 2023-07-17T19:21:48Z

Validation looks good with get frame rate, get video data format and set fps api's.

Signed-off-by: Darr, Christian <christian.darr@intel.com>

presatish

Posting first set of my review comments.

internal/driver/constants.go

internal/driver/metadata.go

cmd/res/profiles/general.usb.camera.yaml

Signed-off-by: Darr, Christian <christian.darr@intel.com>

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

Signed-off-by: Darr, Christian <christian.darr@intel.com>

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

Signed-off-by: Darr, Christian <christian.darr@intel.com>

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

…ce-usb-camera into set-fps-v4l

ajcasagrande

Still looking at driver/metadata.go. Here are my thoughts so far.

cmd/res/profiles/general.usb.camera.yaml

docs/USB-Camera-Collection.postman_collection.json

internal/driver/device.go

internal/driver/driver.go

ajcasagrande · 2023-07-25T23:45:46Z

internal/driver/driver.go

+			if edgexErr != nil {
+				return errors.NewCommonEdgeXWrapper(edgexErr)
+			}
+			fpsValueDenominator, ok := fpsParam.(map[string]interface{})[FpsValueDenominator]


the naming of this in the API input can be a little confusing. technically these are the "interval" values which are an inverse of the actual fps. its actually spf (sunblock anyone)? seconds per frame. I think maybe we name it FpsInvervalDenominator? Either way we probably need to document it.

Yeah that was a problem during development, the fact that internally it's handled opposite of how we usually think about frame rates.

internal/driver/metadata.go

Signed-off-by: Darr, Christian <christian.darr@intel.com>

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

Signed-off-by: Darr, Christian <christian.darr@intel.com>

internal/driver/driver.go

Signed-off-by: Darr, Christian <christian.darr@intel.com>

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

internal/driver/device.go

internal/driver/metadata.go

internal/driver/device.go

Signed-off-by: Darr, Christian <christian.darr@intel.com>

internal/driver/metadata.go

internal/driver/device.go

internal/driver/driver.go

Signed-off-by: Darr, Christian <christian.darr@intel.com>

vyshali-chitikeshi · 2023-07-27T00:46:37Z

Validation looks good

presatish

LGTM

ChristianDarr-personal added 5 commits July 11, 2023 15:25

feat: setFPS command

e1e864e

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: update set fps command

d624f1d

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: update get commands for fps

3f1845d

Signed-off-by: Darr, Christian <christian.darr@intel.com>

docs: update postman collection

ba099dd

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: allow for fractions

aebb389

Signed-off-by: Darr, Christian <christian.darr@intel.com>

ChristianDarr-personal force-pushed the set-fps-v4l branch from e91ec58 to aebb389 Compare July 14, 2023 21:15

docs: update postman

16256f2

Signed-off-by: Darr, Christian <christian.darr@intel.com>

ChristianDarr-personal changed the title ~~Set fps v4l~~ feat: SetFps v4l2 + ffmpeg command Jul 14, 2023

ChristianDarr-personal added 3 commits July 19, 2023 13:41

docs: update name

27c27c2

Signed-off-by: Darr, Christian <christian.darr@intel.com>

docs: update postman

d508cd4

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: update setfps capabilities

8b42b1c

Signed-off-by: Darr, Christian <christian.darr@intel.com>

ChristianDarr-personal force-pushed the set-fps-v4l branch from f176b3a to 8b42b1c Compare July 19, 2023 18:42

ChristianDarr-personal requested a review from presatish July 19, 2023 19:02

feat: allow path index query parameter

dfd140b

Signed-off-by: Darr, Christian <christian.darr@intel.com>

ChristianDarr-personal requested a review from ajcasagrande July 20, 2023 20:07

feat: cleanup

fe40acc

Signed-off-by: Darr, Christian <christian.darr@intel.com>

presatish requested changes Jul 21, 2023

View reviewed changes

ChristianDarr-personal and others added 11 commits July 21, 2023 13:00

refactor: frame rate names

1f5bcfc

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: update frame rate name

8d225cc

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: add check for query parameter

7d14de6

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: update copyright for some files

e350c39

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

fix: add check for read commands

6261b90

Signed-off-by: Darr, Christian <christian.darr@intel.com>

docs: update postman

21d34cd

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: change query parameter

83e9707

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: updated an error message to make it more user friendly

d171066

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

fix: purge reqs cache

63662aa

Signed-off-by: Darr, Christian <christian.darr@intel.com>

fix: change attributes clear command

95fcb6b

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: update query parameter handling

ec97e7c

Signed-off-by: Darr, Christian <christian.darr@intel.com>

presatish added 2 commits July 25, 2023 13:21

feat: clean up of code

7225df0

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

Merge branch 'set-fps-v4l' of github.com:EdgeX-Camera-Management/devi…

8afefe1

…ce-usb-camera into set-fps-v4l

ajcasagrande requested changes Jul 25, 2023

View reviewed changes

ajcasagrande reviewed Jul 25, 2023

View reviewed changes

internal/driver/metadata.go Outdated Show resolved Hide resolved

ajcasagrande reviewed Jul 25, 2023

View reviewed changes

internal/driver/metadata.go Outdated Show resolved Hide resolved

ajcasagrande reviewed Jul 25, 2023

View reviewed changes

internal/driver/metadata.go Show resolved Hide resolved

ChristianDarr-personal and others added 3 commits July 25, 2023 19:56

feat: address pr comments

82c8a5e

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: modified description of a command

8fc6a65

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

feat: refactor function names

fe46ff3

Signed-off-by: Darr, Christian <christian.darr@intel.com>

presatish reviewed Jul 26, 2023

View reviewed changes

internal/driver/driver.go Outdated Show resolved Hide resolved

presatish reviewed Jul 26, 2023

View reviewed changes

internal/driver/driver.go Outdated Show resolved Hide resolved

feat: refactor fps names and frameformat command

e31c184

Signed-off-by: Darr, Christian <christian.darr@intel.com>

ChristianDarr-personal requested review from ajcasagrande and presatish July 26, 2023 19:00

feat: modified some comments and error messages in deriver.go

1b29e38

Signed-off-by: preethi-satishcandra <preethi.satishchandra@intel.com>

ajcasagrande requested changes Jul 26, 2023

View reviewed changes

internal/driver/device.go Outdated Show resolved Hide resolved

internal/driver/device.go Outdated Show resolved Hide resolved

internal/driver/metadata.go Outdated Show resolved Hide resolved

internal/driver/metadata.go Outdated Show resolved Hide resolved

ajcasagrande reviewed Jul 26, 2023

View reviewed changes

internal/driver/device.go Outdated Show resolved Hide resolved

ChristianDarr-personal added 2 commits July 26, 2023 16:25

feat: refactor for output framerate and device closing

bb936e8

Signed-off-by: Darr, Christian <christian.darr@intel.com>

fix: name of fps field

17c9c1b

Signed-off-by: Darr, Christian <christian.darr@intel.com>

ChristianDarr-personal requested a review from ajcasagrande July 26, 2023 22:28

ajcasagrande requested changes Jul 26, 2023

View reviewed changes

ChristianDarr-personal added 2 commits July 26, 2023 18:59

feat: address pr comments name changes

defad8f

Signed-off-by: Darr, Christian <christian.darr@intel.com>

feat: update variable names

db557b0

Signed-off-by: Darr, Christian <christian.darr@intel.com>

ChristianDarr-personal requested a review from ajcasagrande July 27, 2023 00:09

fix: name of deviceCommands

cf72471

Signed-off-by: Darr, Christian <christian.darr@intel.com>

presatish approved these changes Jul 27, 2023

View reviewed changes

ajcasagrande approved these changes Jul 27, 2023

View reviewed changes

ChristianDarr-personal merged commit 040a5a6 into edgexfoundry:main Jul 27, 2023

Conversation

ChristianDarr-personal commented Jul 13, 2023 • edited by presatish Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Testing Instructions

New Dependency Instructions (If applicable)

Uh oh!

codecov-commenter commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vyshali-chitikeshi commented Jul 17, 2023

Uh oh!

presatish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ajcasagrande left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ajcasagrande Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

ChristianDarr-personal Jul 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vyshali-chitikeshi commented Jul 27, 2023

Uh oh!

presatish left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ChristianDarr-personal commented Jul 13, 2023 •

edited by presatish

Loading

codecov-commenter commented Jul 14, 2023 •

edited

Loading