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.

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

v4l2multi_stream_mmal low res mjpeg to frontend #2128

Merged
merged 6 commits into from
Oct 9, 2019

Conversation

popoviciri
Copy link
Contributor

@popoviciri popoviciri commented Oct 6, 2019

This PR simply brings the mjpeg options in the frontend, when using FNC v4l2mmal_multi_stream app written by @jasaw. These are mjpegbitrate, mjpegwidth, mjpegheight and mjpegframerate complementing #2120. A screenshot:
image
Recommended merging order

  1. update motion #2117
  2. v4l2multi_stream_mmal based rtsp fnc #2126
  3. This PR (there will be conflicts to sort out, some additional changes to streameye.sh I couldn't resist).

Looking forward to have this implemented upstream.
Cheers!

@jasaw
Copy link
Collaborator

jasaw commented Oct 7, 2019

@popoviciri I have just pushed a change to v4l2multi_stream_mmal mjpeg bitrate option. Previously the specified mjpeg bitrate was used directly in the mjpeg encoder which is very different from the mjpeg output stream bitrate. Now the mjpeg bitrate actually sets the output bitrate, and the internal mjpeg encoder bitrate is calculated from mjpeg_bitrate*video_framerate/mjpeg_framerate, and it's clipped at a maximum of 25Mbps.

Unfortunately, this affects your PR, which sets the default mjpeg bitrate at 2000000. A new default of 800000 is a good start. Sorry about that.

@popoviciri
Copy link
Contributor Author

Thanks @jasaw. I changed the defaults and consolidated the values across the two files.
I also pushed a commit where the --intra parameter is calculated from the framerate (double) as you suggested. This needs bc which although it is not in the default config, it is being installed by one of the apps so it works.
If you think is too risky to break later (say bc will not always be installed) let me know and I revert this to let --intra 50 hardcoded.
Cheers!

Comment on lines 127 to 128
let "GOP=$((video_framerate))*2"
raspimjpeg_opts="${raspimjpeg_opts} --intra $GOP ${mjpeg_opts}"
Copy link
Collaborator

@jasaw jasaw Oct 7, 2019

Choose a reason for hiding this comment

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

We should probably check whether intra is specified in raspimjpeg.conf file before setting a default of framerate*2 because line 94 will pick up the intra config.

@@ -59,7 +59,7 @@
('incandescent', 'Incandescent'),
('flash', 'Flash'),
('horizon', 'Horizon'),
('greyworld', 'Greyworld')
('greyworld', 'Greyworld')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a nitpick... I would remove the trailing spaces.

@jasaw
Copy link
Collaborator

jasaw commented Oct 7, 2019

This PR looks good. I like the idea of defaulting the intra to 2*framerate, just need to check against raspimjpeg.conf file as mentioned.

Thanks for your hard work !

@jasaw
Copy link
Collaborator

jasaw commented Oct 7, 2019

@popoviciri I have reverted streameye.sh changes from my PR in favour of yours to avoid unnecessary merge conflict.

- check whether intra is specified in raspimjpeg.conf; if NOT, then intra = 2 * framerate
- check is v4l2loopback module is loaded before trying to remove it
@popoviciri
Copy link
Contributor Author

Hi @jasaw. This is in fact your work. I merely made minor changes to what you already pushed here. Anyway, I made the required changes and added a check for v4l2loopback module before calling rmmod. Thanks again!

@jasaw
Copy link
Collaborator

jasaw commented Oct 9, 2019

PR looks good. @ccrisan Have you reviewed this PR?

@ccrisan
Copy link
Collaborator

ccrisan commented Oct 9, 2019

Looks good to me. Let's have it merged :)

@ccrisan ccrisan closed this Oct 9, 2019
@ccrisan ccrisan reopened this Oct 9, 2019
@ccrisan ccrisan merged commit 5490d23 into motioneye-project:dev Oct 9, 2019
@popoviciri popoviciri deleted the mmal_rtsp_frontend branch October 10, 2019 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants