Re: [Gambas-devel] v4l2 patch for gb.v4l
Brought to you by:
gambas
Menu
▾
▴
Re: [Gambas-devel] v4l2 patch for gb.v4l
|
From: Ron_1st <ro...@ti...> - 2009-02-20 17:20:07
|
On Friday 20 February 2009, Gareth Bult wrote:
> >Still the problems with unknown v4l2 formats.
>
> What is the problem?
This is the problem!!
switch(format)
{
case V4L2_PIX_FMT_RGB332: gv4l2_debug("RGB332"); break;
case V4L2_PIX_FMT_RGB444: gv4l2_debug("RGB444"); break;
case V4L2_PIX_FMT_RGB555: gv4l2_debug("RGB555"); break;
case V4L2_PIX_FMT_RGB565: gv4l2_debug("YRGB565"); break;
case V4L2_PIX_FMT_RGB555X: gv4l2_debug("YRGB555X");break;
case V4L2_PIX_FMT_RGB565X: gv4l2_debug("RGB565X"); break;
case V4L2_PIX_FMT_BGR24: gv4l2_debug("BGR24"); break;
case V4L2_PIX_FMT_RGB24: gv4l2_debug("RGB24"); break;
case V4L2_PIX_FMT_BGR32: gv4l2_debug("BGR32"); break;
case V4L2_PIX_FMT_RGB32: /* DEFAULT - NO CONV */ break;
case V4L2_PIX_FMT_GREY: gv4l2_debug("GREY"); break;
case V4L2_PIX_FMT_Y16: gv4l2_debug("Y16"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_PAL8: gv4l2_debug("PAL8"); break;
case V4L2_PIX_FMT_YVU410: gv4l2_debug("YVU410"); break;
case V4L2_PIX_FMT_YVU420: gv4l2_debug("YVU420"); break;
case V4L2_PIX_FMT_YUV420:
//gv4l2_debug("YUV420");
yuv420p_to_rgb (start,THIS->frame,w,h,3);
return;
case V4L2_PIX_FMT_YUYV:
//gv4l2_debug("YUYV");
convert_yuv_to_rgb_buffer(start,THIS->frame,w,h);
return;
case V4L2_PIX_FMT_UYVY: gv4l2_debug("UYVY"); break;
case V4L2_PIX_FMT_YUV422P: gv4l2_debug("YUV422P"); break;
case V4L2_PIX_FMT_YUV411P: gv4l2_debug("YUV411P"); break;
case V4L2_PIX_FMT_Y41P: gv4l2_debug("Y41P"); break;
case V4L2_PIX_FMT_YUV444: gv4l2_debug("YUV444"); break;
case V4L2_PIX_FMT_YUV555: gv4l2_debug("YUV555"); break;
case V4L2_PIX_FMT_YUV565: gv4l2_debug("YUV565"); break;
case V4L2_PIX_FMT_YUV32: gv4l2_debug("YUV32"); break;
case V4L2_PIX_FMT_NV12: gv4l2_debug("NV12"); break;
case V4L2_PIX_FMT_NV21: gv4l2_debug("NV21"); break;
case V4L2_PIX_FMT_YUV410: gv4l2_debug("YUV410"); break;
case V4L2_PIX_FMT_YYUV: gv4l2_debug("YYUV"); break;
case V4L2_PIX_FMT_HI240: gv4l2_debug("HI240"); break;
case V4L2_PIX_FMT_HM12: gv4l2_debug("HM12"); break;
case V4L2_PIX_FMT_SBGGR8: gv4l2_debug("SBGGR8"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_SGBRG8: gv4l2_debug("SBGRG8"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_SBGGR16: gv4l2_debug("SBGGR16"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_MJPEG: gv4l2_debug("MJPEG"); break;
case V4L2_PIX_FMT_JPEG: gv4l2_debug("JPEG"); break;
case V4L2_PIX_FMT_DV: gv4l2_debug("DV"); break;
case V4L2_PIX_FMT_MPEG: gv4l2_debug("MPEG"); break;
case V4L2_PIX_FMT_WNVA: gv4l2_debug("WNVA"); break;
case V4L2_PIX_FMT_SN9C10X: gv4l2_debug("SN9C10X"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_PWC1: gv4l2_debug("PWC1"); break;
case V4L2_PIX_FMT_PWC2: gv4l2_debug("PWC2"); break;
case V4L2_PIX_FMT_ET61X251: gv4l2_debug("ET61X251");break;
case V4L2_PIX_FMT_SPCA501: gv4l2_debug("SPCA501"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_SPCA505: gv4l2_debug("SPCA505"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_SPCA508: gv4l2_debug("SPCA508"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_SPCA561: gv4l2_debug("SPCA561"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_PAC207: gv4l2_debug("PAC207"); break;
case V4L2_PIX_FMT_PJPG: gv4l2_debug("PJPG"); break;
case V4L2_PIX_FMT_YVYU: gv4l2_debug("YVYU"); break;
default:
gv4l2_debug("Frame in unknown format");
break;
}
memcpy(THIS->frame,start,size);
}
To make those formats available for your switch() you put _your_ videodev.h as
videodev2.h in the gambas component instead of the gv4l2.h where it belongs IMHO
or gv4l2.c code.
>
> >The include for the inserted videodev.h should be in the C source that needs it,
> >not the gambas component that uses the C code generated object.
>
> Feel free to modify the location of the "#include" if you think it could be better placed (!)
>
> >This way the gambas commponent can't replaced by some other component code
> >because the .c is more or less corrupt IMHO.
>
> The v4l2 code is in gv4l2.c ,
Yes and related *.h files should they not be included in the gv4l2.h file
when values or anything else from videodev(2).h is used in gv4l2 and not in
code that depends on the ressult of gbv4l2, or do I miss something?
To be able to compile gbv4l2 I do not need CWebCam component?
> the code is CWebCam.* is pre-existing code that was modified to cope with v4l2.
Yes I know and appriciate your work/time spend you did with it.
> I have commented a couple of times that it would be nice to re-factor all the v4l code out into (for example) gv4l.c and leave CWebCam.c purely as a Gambas component container rather than something that contains v4l[2] specific code.
Good idea but why include a *.h file you need in v4l2 code in the component where it is never is used?
Does not sound logic for me.
>
> Please can you specify whether your problem is with gv4l2.c, in which case I can look at "uncorrupting" it .. or with "CWebCam.*" in which case I need some agreement before I completely rip apart someone else's code.
I do not have problems in the CWebCam code, its more or less perfect.
>
> >The best would be not to check fantasy formats not supported and
> >tell the value used in the case into the error message.
>
> Yes, I know, I have a habit of inventing completely non-existent video formats all on my own(!)
> Not!
You did missunderstand this. I do not say _you_ create the fantasy formats.
The fancy formats are the not actual supported formats but are in your videodev.h
as i.e. the V4L2_PIX_FMT_SPCA501, V4L2_PIX_FMT_SPCA505 and V4L2_PIX_FMT_SPCA508
The are put by the ubuntu8.10 team and not the ubunto8.04 or suse team.
If you did support these formats I could understand you did add them.
>
> We have three choices as I see it;
> a. Support the author's camera only, as with the pre-existing v4l code
> b. Support all camera's on the V4L2 spec
> c. Support all camera's that Linux supports (which is a super-set of (b)
>
> I have opted for (c),
I agree This is good
> you are telling me this is wrong
No I do not!!
But be carefull not all distributions support more then the V4L2 spec.
> - I'm open to comments, however as a Linux user, IMHO (c) is a better option.
Yes but a side note is as "ubuntu8.10 linux" user.
Other users using ubuntu8.04 LTS does just that because the LongTermSupport.
> All the video formats listed in the code exist as far as the Linux kernel is concerned. Technically we could have some #ifdef's in the source to look at the kernel version at compile time .. however given the videodev2.h file should be backwards compatible, including it in SVN does make a certain degree of sense. Another alternative would be to remove videodev2.h from SVN and get developers to upgrade to current kernel releases (!)
>
> >tell the value used in the case into the error message.
>
> This is strange, you've obviously looked at the code... you will see in gv4l2_process_image some code that looks like this;
Yes I did :)
>
> switch(format)
> {
> case V4L2_PIX_FMT_RGB332: gv4l2_debug("RGB332"); break;
> case V4L2_PIX_FMT_RGB444: gv4l2_debug("RGB444"); break;
> ...
and these
case V4L2_PIX_FMT_SBGGR8: gv4l2_debug("SBGGR8"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_SGBRG8: gv4l2_debug("SBGRG8"); break; <--- not in all videodev.h
case V4L2_PIX_FMT_SBGGR16: gv4l2_debug("SBGGR16"); break; <--- not in all videodev.h
>
> and a routine that looks like this;
>
> void gv4l2_debug( char *s )
> {
> if( ! gv4l2_debug_mode ) return;
> printf("gambas v4l2: %s [%d]\n",s,errno);
> fflush(stdout);
> }
Good but does not work here in all cases.
>
> So assuming you have debug turned on .. it "should" tell you what format the camera is supplying if it can't handle it ...
> Feel free to enhance the text of the message if you wish .. does this not work for you ?
>
The point is "case V4L2_PIX_FMT_SBGGR8:" is tested by the compiler before "gv4l2_debug("SBGGR8"); break;"
It is the case where the error occurs not the code to run if the case matches, that is proven by
the existing formats declared in videodev.h file.
(I assume the compiler checks the whole line and find undeclared value for the case and trigger the error
before the real translating to object(assembly) code.)
You do only support:
case V4L2_PIX_FMT_YUV420:
//gv4l2_debug("YUV420");
yuv420p_to_rgb (start,THIS->frame,w,h,3);
return;
case V4L2_PIX_FMT_YUYV:
//gv4l2_debug("YUYV");
convert_yuv_to_rgb_buffer(start,THIS->frame,w,h);
return;
default:
gv4l2_debug("Frame in unknown format");
break;
My suggestion is simple
switch(format)
{
case V4L2_PIX_FMT_YUV420:
//gv4l2_debug("YUV420");
yuv420p_to_rgb (start,THIS->frame,w,h,3);
return;
case V4L2_PIX_FMT_YUYV:
//gv4l2_debug("YUYV");
convert_yuv_to_rgb_buffer(start,THIS->frame,w,h);
return;
default:
gv4l2_debug("Frame in unknown format" & format);
/* concat the string with the format used in switch() */
break;
}
This wil do the same as you do now for the unsupported formats.
The formats that are not supported in videodev.h (old or new versions)
do not raise compile time errors.
Proof CWebcam is OK is given by the fact that remove or remark the
lines with unknown case values do not give compile errors.
I do understand you did include all formats in the case for testing
and may be as hints to support these formats in future but now
these case lines are active with wrong values on some other boxes
(distribution(versions)) as yours.
switch (car)
{
case car_opel : print "opel" :break
case car_ford : print "ford" :break
case Car_honda : paint_the_car_blue(honda) :return;
case car_bently : print "bently" break
default
print "unknown car"
break;
}
What does happen if car_ford is not definied in the .h file?
Here the compiler gives a valid error no matter what should be done
as print or paint.
IMHO gv4l2.c should compile without CWebCam and then gv4l2.c it need the reference to videodev(2).h
And now I want to drop CWebCam and go greate CWebCam3
CWebCam3 only need gbv4l2.
The way your videodev2.h is included now force me to include
CWebCam and this result in conflicting functions. double declared.
BTW the only correct videodev.h should be the one supplied by the distribution.
If you _did_ support the extra formats in your version I can understand
when you a include 'private'(ubuntu8.10) version, but just those extra formats are not supported.
The intention was just a friendly question to correct the include place
and a suggestion to prevent compile time errors.
If you think it is a attack from me?
Not!
I know I can learn a lot from Benoit and yes also from you but here
I think I'm not totaly wrong.
I hope Benoit will take a look if I'm possible wrong.
> Gareth.
>
Best regards,
Ron_1st
--
P.S.
Flash supports only v4l and not v4l2 cams ATM :(
I need and have a v4l2 to v4l converter to get my v4l2 devices working.
Convertor is Flashcam with vloopback as support.
|