Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix video stream demo image scaling, shaking and color shift #40

Merged
merged 5 commits into from
Apr 14, 2022
Merged

Fix video stream demo image scaling, shaking and color shift #40

merged 5 commits into from
Apr 14, 2022

Conversation

nunoguedelha
Copy link
Collaborator

Fixes #39 .

@nunoguedelha
Fix 'Stream Video' demo image scaling
f49afae
@nunoguedelha
Fix 'Stream Video' demo image shaking
b3d0a53 
- The image was trembling due to the display of the HTML element 'write-time'. This element is the image frame refresh interval oscillating between ~9 and ~11 ms, which results in a element length toggling.
- As the image has a display 'inline-block' style, it flickers along with the 'write-time' length.
- We just use a line break to avoid this effect.
@nunoguedelha nunoguedelha self-assigned this Apr 4, 2022
@nunoguedelha nunoguedelha mentioned this pull request Apr 4, 2022
Closed
@nunoguedelha
Fix color channel order in the jpeg compression input image
013c926 
Refer to issue 41.
- OpenCV library assumes the color channel order to be BGR or BGRA.
- before calling `cv::imencode()`, use `cv::cvtColor()` for converting the original
  channel order into BGR or BGRA.
- an exception is raised for non supported pixel codes.
@nunoguedelha
Implement custom exception for color conversion code map
7aedb86
@nunoguedelha nunoguedelha marked this pull request as ready for review April 14, 2022 02:40
@nunoguedelha nunoguedelha requested a review from traversaro April 14, 2022 02:41
@nunoguedelha
Copy link
Collaborator Author

CC @S-Dafarra @RiccardoGrieco

@nunoguedelha
Copy link
Collaborator Author

I'm still finishing an approach based on @S-Dafarra's suggestion, but it's already worth reviewing the implementation so far already discussed in the call.

traversaro
traversaro approved these changes Apr 14, 2022
View reviewed changes
Copy link
Member

@traversaro traversaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, seems ok. Not sure if you want to check if we can do just with YARP but anyhow we can always merge this and improve it later.

@nunoguedelha
Copy link
Collaborator Author

Thanks, seems ok. Not sure if you want to check if we can do just with YARP but anyhow we can always merge this and improve it later.

Thanks @traversaro , I will submit a separate PR for the improvement, as you suggested, that way it shall be easier to discuss around it and eventually discard it.

@nunoguedelha nunoguedelha mentioned this pull request Apr 14, 2022
Closed
@nunoguedelha nunoguedelha merged commit f100ce6 into master Apr 14, 2022
@nunoguedelha nunoguedelha deleted the fix/videoStreamDemo-image-scaling-shaking-colorShift branch April 14, 2022 20:33
This was referenced Apr 14, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RiccardoGrieco RiccardoGrieco RiccardoGrieco left review comments

@traversaro traversaro traversaro approved these changes

Assignees

@nunoguedelha nunoguedelha

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stream video example fails with the Framegrabber bouncing ball
3 participants
@nunoguedelha @traversaro @RiccardoGrieco