You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/03/05 13:59:20 UTC
[GitHub] [incubator-nuttx] patacongo opened a new issue #439: Video: Correct
use of global variable
patacongo opened a new issue #439: Video: Correct use of global variable
URL: https://github.com/apache/incubator-nuttx/issues/439
Reported to me via email:
The v4l2-like layer (video.c) uses a global variable inappropriately. A global variable is used instead of creating a struct in /boards/ and passing it. So we have g_video_devops->set_buf(...) for example
g_video_devops being declared in video.h: FAR const struct video_devops_s *g_video_devops;
And being used in the CXD56xx board here: boards/arm/cxd56xx/spresense/src/cxd56_bringup.c: g_video_devops = isx012_initialize();
Wouldn't it be better to create the pointer in boards and pass it as argument ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] jerpelea commented on issue #439: Video: Correct
use of global variable
Posted by GitBox <gi...@apache.org>.
jerpelea commented on issue #439: Video: Correct use of global variable
URL: https://github.com/apache/incubator-nuttx/issues/439#issuecomment-595275166
I am looking into it ASAP
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #439: Video: Correct use of global variable
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #439:
URL: https://github.com/apache/incubator-nuttx/issues/439#issuecomment-642640881
A related issus the the use, or rather the lacke of the use of the video driver initialization function: video_initialize().
That function should be called from the board bringup() functions but it is not. In fact, it is not called from anywhere in the OS. Searching a little, we find that it is insanely called from the Sony SDK applications directly. That is very bad design: Calling directly into the OS from application bypassing the portable, POSIX interface is strictly forbidden. From the INVIOLABLES.txt:
Strict POSIX compliance
-----------------------
o Strict conformance to the portable standard OS interface as defined at
OpenGroup.org.
o The portable interface must never be compromised only for the sake of
expediency.
o Expediency or even improved performance are not justifications for
violation of the strict POSIX interface.
My recommendation is:
1. Remove the direct call to video_initialize() from the SDK application. It is not needed.
2. Add the call to video_initialize() to the spresense bringup() logic.
3. Remove the (now extern'ed) global variable g_video_devops from include/nuttx/video/video.h and mark the g_video_devops global variable in drivers/video/video.c as static
4. Add a : FAR const struct video_devops_s *devops argument to video_initialize()
5. Provide the video_devops_s instance to the driver with video_initialize() in spresense bringup() function.
The current situation is intolerable and must be corrected!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo closed issue #439: Video: Correct use of global variable
Posted by GitBox <gi...@apache.org>.
patacongo closed issue #439:
URL: https://github.com/apache/incubator-nuttx/issues/439
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #439: Video: Correct use of global variable
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #439:
URL: https://github.com/apache/incubator-nuttx/issues/439#issuecomment-642640881
A related issus the the use, or rather the lacke of the use of the video driver initialization function: video_initialize().
That function should be called from the board bringup() functions but it is not. In fact, it is not called from anywhere in the OS. Searching a little, we find that it is insanely called from the Sony SDK applications directly. That is very bad design: Calling directly into the OS from application bypassing the portable, POSIX interface is strictly forbidden. From the INVIOLABLES.txt:
Strict POSIX compliance
-----------------------
o Strict conformance to the portable standard OS interface as defined at
OpenGroup.org.
o The portable interface must never be compromised only for the sake of
expediency.
o Expediency or even improved performance are not justifications for
violation of the strict POSIX interface.
My recommendation is:
1. Remove the direct call to video_initialize() from the SDK application. It is not needed.
2. Add the call to video_initialize() to the spresense bringup() logic.
3. Remove the (now extern'ed) global variable g_video_devops from include/nuttx/video/video.h and mark the g_video_devops global variable in drivers/video/video.c as static
4. Add a : FAR const struct video_devops_s *devops argument to video_initialize()
5. Provide the video_devops_s instance to the driver with video_initialize() in the spresense bringup() function.
The current situation is intolerable and must be corrected!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jerpelea commented on issue #439: Video: Correct use of global variable
Posted by GitBox <gi...@apache.org>.
jerpelea commented on issue #439:
URL: https://github.com/apache/incubator-nuttx/issues/439#issuecomment-644020013
@patacongo can we close the isssue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jerpelea commented on issue #439: Video: Correct use of global variable
Posted by GitBox <gi...@apache.org>.
jerpelea commented on issue #439:
URL: https://github.com/apache/incubator-nuttx/issues/439#issuecomment-642681571
I am sorry for the long response time and thanks for the suggestion.
I will implement and test the suggestion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jerpelea edited a comment on issue #439: Video: Correct use of global variable
Posted by GitBox <gi...@apache.org>.
jerpelea edited a comment on issue #439:
URL: https://github.com/apache/incubator-nuttx/issues/439#issuecomment-644020013
@patacongo can we close the issue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org