[Ffmpeg-devel] Screen frame grabbing for Win32 platform

[Ramiro Polla]

Ok, just a few more remarks about your patch

GISQUET Christophe wrote:
> +  --enable-win32grab) win32grab="yes";
> +  ;;
>   
IMHO it should be default in Windows (It depends on what the developers 
say though). X11grab has that because it's GPL'd.
> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c	(revision 7791)
> +++ ffmpeg.c	(working copy)
> @@ -3229,10 +3229,14 @@
>  
>      if (has_video) {
>          AVInputFormat *fmt1;
> +
>   
Be careful. Don't send cosmetic changes in your patches.

>  #warning FIXME: find a better interface
>          if(video_device&&!strncmp(video_device,"x11:",4)) {
>              video_grab_format="x11grab";
>          }
> +        else if (video_device&&!strncmp(video_device,"gdi:",4)) {
> +            video_grab_format="win32grab";
> +        }
>   
Anyways, any change to ffmpeg.c will be dropped with the new grab 
interface.
> +/** Win32 grab device demuxer context */
> +typedef struct
> +{
>   
[...]

> +    int     size;          /**< Size in bytes of the grab frame */
> +    int     width;         /**< Width of the grab frame */
> +    int     height;        /**< Height of the grab frame */
> +    int     bpp;           /**< Bits per pixel of the grab frame */
>   
I used a BITMAPINFOHEADER to keep all this. Maybe you should keep the 
result of your GetObject.

> +    switch (s->bpp)
> +    {
> +    case 8: input_pixfmt = PIX_FMT_PAL8; break;
> +    case 16: input_pixfmt = PIX_FMT_RGB555; break;
> +    case 24: input_pixfmt = PIX_FMT_BGR24; break;
> +    case 32: input_pixfmt = PIX_FMT_RGBA32; break;
> +    default:
> +        av_log(s1, AV_LOG_ERROR, "image depth %i not supported ... aborting\n",
> +               s->bpp);
> +        return -1;
> +    }
>   
I left this to RAWVIDEO. It doesn't return an error though. Also, 
RAWVIDEO deals with bottom-down DIBS if you specify bits_per_sample.

> +    av_set_pts_info(st, 64, 1, 1000000); /* 32 bits pts in ms */
>   
Then set it to 32 bits instead of 64, and in ms instead of us.
> +    s->time_base  = ap->time_base;
> +    s->time_frame = av_gettime() / av_q2d(ap->time_base);
>   
This isn't necessary to keep around in the context. We shouldn't be 
calculating the time anyways. Let FFmpeg do that

> +    /* Calculate the time of the next frame */
> +    s->time_frame += INT64_C(1000000);
> +
> +    /* wait based on the frame rate */
> +    for(;;) {
> +        curtime = av_gettime();
> +        delay = s->time_frame * av_q2d(s->time_base) - curtime;
> +        if (delay <= 0) {
> +            if (delay < INT64_C(-1000000) * av_q2d(s->time_base)) {
> +                s->time_frame += INT64_C(1000000);
> +            }
> +            break;
> +        }
> +        Sleep(delay/1000);
> +    }
> +
>   
as in using the rate emulation option. ffmpeg -r 10 -re -f gdi -i 0,0
sleeps the necessary time. Also, use the 32-bit result from 
GetTickCount, since that's what will be used internally anyways. (Hmm... 
on second thoughs, I don't know well the developers would like this)
> Property changes on: libavformat\win32grab.c
> ___________________________________________________________________
> Name: svn:eol-style
>    + native
>   
Be careful =) Don't change the eol-style.

Also, we need to capture the mouse pointer...

Ramiro Polla