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

Add the support for the last drivers of Raspberry Pi 3 Model B Plus Rev 1.3 #23

Closed
wants to merge 2 commits into from

Conversation

geoffreyp
Copy link

No description provided.

Copy link
Contributor

@gregnr gregnr left a comment

Choose a reason for hiding this comment

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

Hi @geoffreyp thanks for the PR. I just left one comment I hope you can help answer.

@@ -40,6 +40,7 @@ export default class StillCamera {
case 'BCM2711':
case 'BCM2835 - Pi 3 Model B':
case 'BCM2835 - Pi 3 Model B+':
case 'Raspberry Pi 3 Model B Plus Rev 1.3':
Copy link
Contributor

Choose a reason for hiding this comment

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

Does model really come back with this string on a rev 1.3? Just seems odd that the naming convention would change just for a Pi 3 rev 1.3.

Copy link
Author

@geoffreyp geoffreyp Nov 16, 2020

Choose a reason for hiding this comment

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

Hello @gregnr !
Yes it is the correct string, I just use the name throw by the error
``Could not determine JPEG signature. Unknown system model '${systemInfo.model}'
and it works for me

@geoffreyp
Copy link
Author

Hello @gregnr !
Yes it is the correct string, I just use the name throw by the error
``Could not determine JPEG signature. Unknown system model '${systemInfo.model}'
and it works for me

@geoffreyp
Copy link
Author

Hello @gregnr, do you need more information before merge the PR?

@gregnr
Copy link
Contributor

gregnr commented Dec 12, 2020

Hi @geoffreyp, sorry for the delay and thanks again for the addition.

We are getting a number of reports like this where the model format has completely changed. Before merging this I would just like to look at the bigger picture to understand what changed.

TBH, we may not even need this check at all, since every model so far has the same JPEG signature. This logic originally existed because people were reporting different JPEG signatures, but this may not have been the right solution. It's possible it should have been based on camera model, not Pi model.

The biggest blocker right now is getting my hands on some of these Pi's + Cameras and verifying the models and JPEG signatures. Perhaps next steps is to create a tool that makes it easy for developers to report different Pi+Camera+JPEG signature combinations.

@gregnr
Copy link
Contributor

gregnr commented Dec 12, 2020

I'll take a deeper look today and follow up here.

@gregnr
Copy link
Contributor

gregnr commented Dec 13, 2020

After further investigation, it appears the Pi model has changed due one of, or the combination of:

  • The systeminformation dep changed the way Pi model is reported. We originally used v4.23.0 of systeminformation (though kept the dependency open to minor and patch updates), and they introduced a breaking change on v4.29.1 without bumping the major version.
  • Newer kernel versions report a different model than before (systeminformation calls /proc/cpuinfo under the hood)

Regardless, we have decided to remove the Pi model check altogether for a couple reasons:

  • The check was originally added because the original JPEG signature didn't work for a number of people (including myself at a later date). We guessed that this was because different Pi models had different JPEG encoders and therefore generated different JPEG signatures. We updated to a newer signature and added the model check so that we could return different signatures in the future as people reported them. It's now been over 9 months and every model so far still uses the same signature. Most likely the signature is not based on model after all, and instead based on something else, like camera model or OS version.
  • If there are multiple possible signatures, a better solution in the future would be to simply support multiple signatures, ie. for each MJPEG data callback, check all JPEG signatures instead of just one.

This change is available now on v0.3.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants