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

Temporary workaround for chrome/skype. #180

Closed
wants to merge 2 commits into from

Conversation

maximilianduell
Copy link

No description provided.

@ivanperez-keera
Copy link

I have questions and comments about this.

First, i can confirm that this works well on linux for skype. Chromium already worked without it. However, it no longer works on Cheese, for example.

Obviously, being a workaround, it is not a good solution. I have several comments:

  • First, I'd say that calling it chrome workaround in the parameter makes it pretty hard to consider seriously (but I'm not affiliated with the project in any way). I'd call it "fix_minimum_setttings" or something that conveys what's going on in a more general way.

  • Second, could it be possible to fix the "minimum" resolution to something higher than 320x240 (which I understand is the current fixed resolution when the parameter is enabled)?

  • Third, could it be possible to change these values from the command line when loading the kernel without having to re-compile it? I think that would be a better "workaround".

@maximilianduell
Copy link
Author

maximilianduell commented Aug 7, 2019

Thanks for these valid comments. In fact, the code I wrote is just a minimal solution to get skype/chrome working again without having to recompile the browser myself.

It should be easy to add additional parameters which allow to change the fixed resolution and framerate also from the command line.

There is also a more extensive solution which I described in an issue in my private repo, namely to expose some selected "preferrable" resolution/framerate settings via the older v4l interface as valid resolutions, so that apps reading the possible resolutions via this API can change between them without even reloading the module with new parameters.

However, as I do not see much resonance on my simple and stupid patch, I think it will not be worth the time to implement these improvements. After all my patch is sufficient as a workaround for my own purposes. If you actually do need those improvements urgently, let me know.

Regarding higher resolutions for skype, I myself had the experience that these will take up a lot of CPU power and battery, running up the fans. Hence I picked the smallest possible resolution.

@ivanperez-keera
Copy link

I think it's just a project that has stalled for a bit, but resonance and push will come from pushing forward from these ends. I would not let the lack of response discourage you from contributing.

I do not speak on behalf of the project, so I do not know what the requirements would be to merge this feature (at least as a first step so that we have some experimental fix). I'd say renaming the parameter could be a reasonable thing to do for a first merge, and making it possible to work for other resolutions could be a separate PR (just to divide it into bite-sized chunks that are almost trivial to merge).

@patjak Do you have an opinion on this? Is this mergeable?

@patjak
Copy link
Owner

patjak commented Aug 21, 2019

Hi, sorry for the delay. I've been thinking about this for a while and perhaps we should just revert back to discrete frame sizes instead. Most applications seems to only care about UVC and UVC only uses discrete frame sizes. Perhaps we can add stepwise back with a kernel parameter. So basically what you are proposing but the other way around :)

I don't like the 320x240 limit. There are both low-end and high-end machines using facetimehd so that would hurt the high-end machines.

@maximilianduell
Copy link
Author

Still thinking?

@gianlucapettinello
Copy link

Any news?

@patjak
Copy link
Owner

patjak commented May 20, 2020

@maximilianduell
Let's revert to discrete frame sizes as default but skip the 320x240 limit. Would you like to prepare a patch for this or should I?

@patjak
Copy link
Owner

patjak commented May 22, 2020

I went ahead and patched this. Thanks

@patjak patjak closed this May 22, 2020
@maximilianduell
Copy link
Author

Ah sorry, just read it now. Thanks!

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.

4 participants