[FFmpeg-devel] [Ffmpeg-devel] [PATCH] video grab support VIDEO_PALETTE_RGB565
yi li
liyi.dev
Tue May 8 10:50:42 CEST 2007
Hi,
Made more changes accordingly. (And sorry for my late response due to
holiday).
> this would be more readable if things where vertically aligned like:
> {.palette = VIDEO_PALETTE_RGB565, .depth = 16, .pix_fmt = PIX_FMT_BGR565},
> {.palette = VIDEO_PALETTE_GREY , .depth = 8, .pix_fmt = PIX_FMT_GRAY8}
> also i think the .palette, .depth, ... hurts readability more than it
helps
I copy the name from v4l header "linux/videodev.h" "struct video_picture"
definition, and would like to keep them compatible.
>> + if (-1 != ioctl(video_fd, VIDIOCSPICT, &pict))
>> + break;
> i think this should rather stay <0 instead of ==-1
I made change in the new patch since I saw the similar "<0" usage in grab.c.
However, "ioctl" mannual states that: "Upon successful completion, ioctl()
shall return a value other than -1 that depends upon the STREAMS device
control function. Otherwise, it shall return -1 and set errno to indicate
the error."
-Yi
On 4/30/07, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> Hi
>
> On Mon, Apr 30, 2007 at 07:30:24PM +0800, yi li wrote:
> > I made some changes accordingly. Hope it is better.
> >
> > Regards,
> > -Yi
> >
> > On 4/29/07, Michael Niedermayer <michaelni at gmx.at> wrote:
> > >
> > >Hi
> > >
> > >On Sun, Apr 29, 2007 at 03:20:03PM +0800, yi li wrote:
> > >> Hi,
> > >>
> > >> A small patch for libavformat/grab.c to make it support v4l device
> with
> > >> "VIDEO_PALETTE_RGB565" format.
> > >> Any comments?
> > >>
> > >> Regards,
> > >>
> > >> -Yi
> > >
> > >> --- grab.c.orig 2007-04-29 15:14:43.000000000 +0800
> > >> +++ grab.c 2007-04-29 15:13:36.000000000 +0800
> > >> @@ -174,8 +174,13 @@
> > >> pict.palette=VIDEO_PALETTE_GREY;
> > >> pict.depth=8;
> > >> ret = ioctl(video_fd, VIDIOCSPICT, &pict);
> > >> - if (ret < 0)
> > >> - goto fail1;
> > >> + if (ret < 0) {
> > >> + pict.palette=VIDEO_PALETTE_RGB565;
> > >> + pict.depth=16;
> > >> + ret = ioctl(video_fd, VIDIOCSPICT, &pict);
> > >> + if (ret < 0)
> > >> + goto fail1;
> > >> + }
> > >> }
> > >
> > >tabs are forbidden in svn
> > >rgb565 should be checked before gray
> > >and ideally the whole should be simplified, a simple table with
> > >VIDEO_PALETTE_*, depth, pix_fmt and frame_size factor could be used
> [...]
>
> > --- grab.c.orig 2007-04-30 13:52:26.000000000 +0800
> > +++ grab.c 2007-04-30 19:21:38.000000000 +0800
> > @@ -53,6 +53,19 @@
> > uint8_t *lum_m4_mem;
> > } VideoData;
> >
> > +struct {
> > + int palette;
> > + int depth;
> > + enum PixelFormat pix_fmt;
> > +} video_formats [] = {
> > + {.palette = VIDEO_PALETTE_YUV420P, .depth = 12, .pix_fmt =
> PIX_FMT_YUV420P},
> > + {.palette = VIDEO_PALETTE_YUV422, .depth = 16, .pix_fmt =
> PIX_FMT_YUYV422},
> > + {.palette = VIDEO_PALETTE_RGB24, .depth = 24,
>
> trailing whitespace is forbidden in svn
>
>
> > + .pix_fmt = PIX_FMT_BGR24}, /* NOTE: v4l uses BGR24, not RGB24 !
> */
> > + {.palette = VIDEO_PALETTE_RGB565, .depth = 16, .pix_fmt =
> PIX_FMT_BGR565},
> > + {.palette = VIDEO_PALETTE_GREY, .depth = 8, .pix_fmt =
> PIX_FMT_GRAY8}
> > +};
>
> this would be more readable if things where vertically aligned like:
>
> {.palette = VIDEO_PALETTE_RGB565, .depth = 16, .pix_fmt = PIX_FMT_BGR565},
> {.palette = VIDEO_PALETTE_GREY , .depth = 8, .pix_fmt = PIX_FMT_GRAY8}
>
> also i think the .palette, .depth, ... hurts readability more than it
> helps
>
>
>
> [...]
> > @@ -158,27 +172,15 @@
> > /* try to choose a suitable video format */
> > pict.palette = desired_palette;
> > pict.depth= desired_depth;
> > - if (desired_palette == -1 || (ret = ioctl(video_fd, VIDIOCSPICT,
> &pict)) < 0) {
> > - pict.palette=VIDEO_PALETTE_YUV420P;
> > - pict.depth=12;
> > - ret = ioctl(video_fd, VIDIOCSPICT, &pict);
> > - if (ret < 0) {
> > - pict.palette=VIDEO_PALETTE_YUV422;
> > - pict.depth=16;
> > - ret = ioctl(video_fd, VIDIOCSPICT, &pict);
> > - if (ret < 0) {
> > - pict.palette=VIDEO_PALETTE_RGB24;
> > - pict.depth=24;
> > - ret = ioctl(video_fd, VIDIOCSPICT, &pict);
> > - if (ret < 0) {
> > - pict.palette=VIDEO_PALETTE_GREY;
> > - pict.depth=8;
> > - ret = ioctl(video_fd, VIDIOCSPICT, &pict);
> > - if (ret < 0)
> > - goto fail1;
> > - }
> > - }
> > + if (desired_palette == -1) {
> > + for (j = 0; j < vformat_num; j++) {
> > + pict.palette = video_formats[j].palette;
> > + pict.depth = video_formats[j].depth;
> > + if (-1 != ioctl(video_fd, VIDIOCSPICT, &pict))
> > + break;
>
> i think this should rather stay <0 instead of ==-1
>
>
> [...]
> > @@ -248,27 +250,18 @@
> > s->frame_format = s->gb_buf.format;
> > s->use_mmap = 1;
> > }
> > -
> > - switch(s->frame_format) {
> > - case VIDEO_PALETTE_YUV420P:
> > - frame_size = (width * height * 3) / 2;
> > - st->codec->pix_fmt = PIX_FMT_YUV420P;
> > - break;
> > - case VIDEO_PALETTE_YUV422:
> > - frame_size = width * height * 2;
> > - st->codec->pix_fmt = PIX_FMT_YUYV422;
> > - break;
> > - case VIDEO_PALETTE_RGB24:
> > - frame_size = width * height * 3;
> > - st->codec->pix_fmt = PIX_FMT_BGR24; /* NOTE: v4l uses BGR24,
> not RGB24 ! */
> > - break;
> > - case VIDEO_PALETTE_GREY:
> > - frame_size = width * height * 1;
> > - st->codec->pix_fmt = PIX_FMT_GRAY8;
> > - break;
> > - default:
> > - goto fail;
> > +
> > + for (j = 0; j < vformat_num; j++) {
> > + if (s->frame_format == video_formats[j].palette) {
> > + frame_size = width * height * video_formats[j].depth /
> sizeof(char);
>
> sizeof(char) = 1
>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No snowflake in an avalanche ever feels responsible. -- Voltaire
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: grab.c.rgb565.2.patch
Type: text/x-patch
Size: 3650 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070508/ef5c7116/attachment.bin>
More information about the ffmpeg-devel
mailing list