[FFmpeg-devel] [PATCH] VFW capture support
Måns Rullgård
mans
Mon Mar 3 07:52:19 CET 2008
Ramiro Polla <ramiro at lisha.ufsc.br> writes:
> Hello,
>
> M?ns Rullg?rd wrote:
> [...]
>>> - I tried to make the code look as much as possible as the code given
>>> in MSDN, so that means I used "LRESULT CALLBACK" without even knowing
>>> what it does or if it can be removed.
>> I'm afraid that's unacceptable in FFmpeg. Please find out what it
>> does.
>
> CALLBACK changed to __stdcall.
OK, I see what it does, and it should be left as CALLBACK, in case
it's not always defined to __stdcall.
>>> I have also used "var == FALSE" and "var == NULL" checks. I prefer
>>> that instead of "!var" just to be closer to MSDN.
>> Using "var == FALSE", and "var == TRUE" more so, not only not our
>> style, it's plain stupid. Being closer to msdn is hardly a desirable
>> goal.
>
> Hey, we never know if Microsoft someday decides that FALSE == 18
> instead of the expected 0...
Fair point.
> You even say it yourself a few lines down from here:
>> Are these values guaranteed to be correct for all Windows versions?
>
> Anyways, I don't really care that much: changed.
>
>>> - Some defines are missing from MinGW include headers. I'll try to get
>>> them integrated into MinGW. After that, we can properly check for
>>> MinGW versions to define them, or just plain remove the definitions
>>> here and require a newer version of MinGW. In the meantime, I think
>>> there's no problem in defining them here.
>> Are these values guaranteed to be correct for all Windows versions?
>
> Probably. If it's not correct, we'll know as soon as someone submits a
> bug report or a patch.
I guess it's acceptable, even if I don't like it. Then again, I don't
like the idea of Windows in the first place.
>>> - If no "-r" parameter is given, way too many frames are read. I
>>> thought FFmpeg was supposed to read the first few frames and decide
>>> the rate based on that. The capture blocks, so the timestamps are
>>> correct, and if some frames were captured and then the frame rate
>>> calculated, it should find the correct frame rate, right?
>> FFmpeg does not estimate frame rate based on elapsed real time,
>> AFAIK.
>
> The question is if it estimates frame rate based on pts. Real time or
> not is irrelevant here.
Of course you're right. I don't know what I was thinking. That said,
can't you set the frame rate in read_header()?
>>> Please review.
>> I am shocked an appalled by the ugly casting required by the win32
>> API, but there's of course nothing we can do about that.
>
> Please refrain from your predictable Windows bashing in the subsequent
> reviews. Thank you.
I'll bash it all I want. It'll still be getting far less than it
deserves.
>>> struct vfw_ctx {
>>> HWND hwnd;
>>> int grabbed;
>>> AVPacket *pkt;
>>> };
>>>
>>> static int vfw_pixfmt( DWORD biCompression )
>> Do we really have to use those dreadful windows typedefs and naming
>> conventions?
>
> I find it best when writing an interface to an API that has
> documentation, the same way you follow variable names from specs.
This function isn't part of any API.
>> It is preferred to have no whitespace immediately inside () in FFmpeg.
>
> This is my preferred style, and I'll maintain this file. You won't
> need to look at it after it gets into SVN.
FFmpeg should use a consistent style.
>>> {
>>> switch( biCompression ) {
>>> case MKTAG( 'Y', 'U', 'Y', '2' ):
>>> return PIX_FMT_YUYV422;
>>> default:
>>> return PIX_FMT_BGR24;
>>> }
>> Is this really complete and correct?
>
> Complete? Most likely not.
> Correct? For me it is...
I'm quite suspicious about the default case. Would it not be better
to list the known values explicitly, and report an error for unknown
values.
> We'll know more as users submit the output from
> ffmpeg -f vfwcap -i 0 -v 99
>
>>> }
>>>
>>> static void dump_bih( AVFormatContext *s, BITMAPINFO *bih )
>>> {
>>> av_log( s, AV_LOG_DEBUG, " biSize:\t%d\n", (int) bih->bmiHeader.biSize );
>>> av_log( s, AV_LOG_DEBUG, " biWidth:\t%d\n", (int) bih->bmiHeader.biWidth );
>>> av_log( s, AV_LOG_DEBUG, " biHeight:\t%d\n", (int) bih->bmiHeader.biHeight );
>>> av_log( s, AV_LOG_DEBUG, " biPlanes:\t%d\n", (int) bih->bmiHeader.biPlanes );
>>> av_log( s, AV_LOG_DEBUG, " biBitCount:\t%d\n", (int) bih->bmiHeader.biBitCount );
>>> av_log( s, AV_LOG_DEBUG, " biCompression:\t%.4s\n", (char*) &bih->bmiHeader.biCompression );
>>> av_log( s, AV_LOG_DEBUG, " biSizeImage:\t%d\n", (int) bih->bmiHeader.biSizeImage );
>>> av_log( s, AV_LOG_DEBUG, " biXPelsPerMeter:\t%d\n", (int) bih->bmiHeader.biXPelsPerMeter );
>>> av_log( s, AV_LOG_DEBUG, " biYPelsPerMeter:\t%d\n", (int) bih->bmiHeader.biYPelsPerMeter );
>>> av_log( s, AV_LOG_DEBUG, " biClrUsed:\t%d\n", (int) bih->bmiHeader.biClrUsed );
>>> av_log( s, AV_LOG_DEBUG, " biClrImportant:\t%d\n", (int) bih->bmiHeader.biClrImportant );
>> All those (int) casts are unnecessary.
>
> warning: format '%d' expects type 'int', but argument 4 has type 'DWORD'
That's because DWORD is 'unsigned long' (at least on 32-bit), so the
format specifier should be %lu.
>> If any of the fields are
>> unsigned, the corresponding format specifiers should be %u.
>
> http://msdn2.microsoft.com/en-us/library/ms532290(VS.85).aspx
> They're defined as DWORDs and LONGs. Some allow negative numbers, the
> rest I don't know but have never seen with signed numbers.
> It's just debugging information. We'll learn more with bug reports.
DWORD is unsigned, and so should use an unsigned format specifier.
Debugging being correct is as important as anything.
> [more predictable Windows bashing]
>
> [Useless (LRESULT) casts]
>
> Removed.
>
>>> pkt->pts = GetTickCount( );
>>> memcpy( pkt->data, vdhdr->lpData, vdhdr->dwBytesUsed );
>> Is it really not possible to capture without this memcpy()?
>
> That's the best I found. The suggested way of capturing VFW frames
> from MSDN involves copying to the clipboard and then to whatever you
> need it for. Maybe someday someone will find some dirty hack that
> avoids it.
Video... clipboard... words do not exist to express my disgust.
> /*
> * VFW capture interface
> * Copyright (c) 2006-2008 Ramiro Polla.
> *
> * This file is part of FFmpeg.
> *
> * FFmpeg is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> * License as published by the Free Software Foundation; either
> * version 2.1 of the License, or (at your option) any later version.
> *
> * FFmpeg is distributed in the hope that it will be useful,
> * but WITHOUT ANY WARRANTY; without even the implied warranty of
> * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> * Lesser General Public License for more details.
> *
> * You should have received a copy of the GNU Lesser General Public
> * License along with FFmpeg; if not, write to the Free Software
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> #include "avformat.h"
> #include <vfw.h>
> #include <windows.h>
>
> /* Defines for VFW missing from MinGW.
> * Remove this when MinGW incorporates them. */
> #define WM_CAP_START (0x0400)
> #define WM_CAP_SET_CALLBACK_FRAME (WM_CAP_START + 5)
> #define WM_CAP_GET_USER_DATA (WM_CAP_START + 8)
> #define WM_CAP_SET_USER_DATA (WM_CAP_START + 9)
> #define WM_CAP_DRIVER_CONNECT (WM_CAP_START + 10)
> #define WM_CAP_DRIVER_DISCONNECT (WM_CAP_START + 11)
> #define WM_CAP_GRAB_FRAME (WM_CAP_START + 60)
> #define WM_CAP_GET_VIDEOFORMAT (WM_CAP_START + 44)
> #define WM_CAP_SET_PREVIEW (WM_CAP_START + 50)
> #define WM_CAP_SET_OVERLAY (WM_CAP_START + 51)
>
> #define HWND_MESSAGE ((HWND)-3)
>
> typedef struct videohdr_tag {
> LPBYTE lpData;
> DWORD dwBufferLength;
> DWORD dwBytesUsed;
> DWORD dwTimeCaptured;
> DWORD dwUser;
> DWORD dwFlags;
> DWORD_PTR dwReserved[4];
> } VIDEOHDR, NEAR *PVIDEOHDR, FAR * LPVIDEOHDR;
> /* End of missing MinGW defines */
Does cygwin have these?
> struct vfw_ctx {
> HWND hwnd;
> int grabbed;
> AVPacket *pkt;
> };
>
> static int vfw_pixfmt( DWORD biCompression )
> {
> switch( biCompression ) {
> case MKTAG( 'Y', 'U', 'Y', '2' ):
> return PIX_FMT_YUYV422;
> default:
> return PIX_FMT_BGR24;
> }
> }
See above.
> static void dump_bih( AVFormatContext *s, BITMAPINFO *bih )
> {
> av_log( s, AV_LOG_DEBUG, " biSize:\t%d\n", (int) bih->bmiHeader.biSize );
> av_log( s, AV_LOG_DEBUG, " biWidth:\t%d\n", (int) bih->bmiHeader.biWidth );
> av_log( s, AV_LOG_DEBUG, " biHeight:\t%d\n", (int) bih->bmiHeader.biHeight );
> av_log( s, AV_LOG_DEBUG, " biPlanes:\t%d\n", (int) bih->bmiHeader.biPlanes );
> av_log( s, AV_LOG_DEBUG, " biBitCount:\t%d\n", (int) bih->bmiHeader.biBitCount );
> av_log( s, AV_LOG_DEBUG, " biCompression:\t%d\t\"%.4s\"\n", (unsigned int)
> bih->bmiHeader.biCompression, (char*) &bih->bmiHeader.biCompression );
> av_log( s, AV_LOG_DEBUG, " biSizeImage:\t%d\n", (int) bih->bmiHeader.biSizeImage );
> av_log( s, AV_LOG_DEBUG, " biXPelsPerMeter:\t%d\n", (int) bih->bmiHeader.biXPelsPerMeter );
> av_log( s, AV_LOG_DEBUG, " biYPelsPerMeter:\t%d\n", (int) bih->bmiHeader.biYPelsPerMeter );
> av_log( s, AV_LOG_DEBUG, " biClrUsed:\t%d\n", (int) bih->bmiHeader.biClrUsed );
> av_log( s, AV_LOG_DEBUG, " biClrImportant:\t%d\n", (int) bih->bmiHeader.biClrImportant );
> }
>
> static LRESULT __stdcall FrameCallbackProc( HWND hwnd, LPVIDEOHDR vdhdr )
> {
> struct vfw_ctx *ctx;
> AVPacket *pkt;
>
> ctx = (struct vfw_ctx *) SendMessage( hwnd, WM_CAP_GET_USER_DATA, 0, 0 );
> if( !ctx )
> return FALSE;
>
> pkt = ctx->pkt;
>
> if( av_new_packet( pkt, vdhdr->dwBytesUsed ) < 0 )
> return FALSE;
>
> pkt->pts = GetTickCount( );
> memcpy( pkt->data, vdhdr->lpData, vdhdr->dwBytesUsed );
>
> ctx->grabbed = 1;
>
> return TRUE;
> }
>
> static int vfw_read_header( AVFormatContext *s, AVFormatParameters *ap )
> {
> struct vfw_ctx *ctx = s->priv_data;
> AVCodecContext *codec;
> AVStream *st;
> int devnum, ret;
> BITMAPINFO *bih;
>
> if( s->flags & AVFMT_FLAG_NONBLOCK ) {
> av_log( s, AV_LOG_ERROR, "Non blocking capture not yet implemented.\n" );
> return AVERROR_IO;
> }
>
> ctx->hwnd = capCreateCaptureWindow( NULL, 0, 0, 0, 0, 0, HWND_MESSAGE, 0 );
> if( !ctx->hwnd ) {
> av_log( s, AV_LOG_ERROR, "Could not create capture window.\n" );
> return AVERROR_IO;
> }
>
> /* If atoi fails, devnum==0 and the default device is used */
> devnum = atoi( s->filename );
>
> ret = SendMessage( ctx->hwnd, WM_CAP_DRIVER_CONNECT, devnum, 0 );
> if( !ret ) {
> av_log( s, AV_LOG_ERROR, "Could not connect to device.\n" );
> return AVERROR_IO;
> }
I'm not sure AVERROR_IO is the proper error code for those failures.
> SendMessage( ctx->hwnd, WM_CAP_SET_OVERLAY, 0, 0 );
> SendMessage( ctx->hwnd, WM_CAP_SET_PREVIEW, 0, 0 );
>
> ret = SendMessage( ctx->hwnd, WM_CAP_SET_CALLBACK_FRAME, 0,
> (LPARAM) FrameCallbackProc );
> if( !ret )
> return AVERROR_IO;
>
> ret = SendMessage( ctx->hwnd, WM_CAP_SET_USER_DATA, 0, (LPARAM) ctx );
> if( !ret )
> return AVERROR_IO;
>
> st = av_new_stream( s, 0 );
> if ( !st )
> return AVERROR_NOMEM;
>
> ret = SendMessage( ctx->hwnd, WM_CAP_GET_VIDEOFORMAT, 0, 0 );
> if( !ret )
> return AVERROR_IO;
> bih = av_malloc( ret );
> if ( !bih )
> return AVERROR_NOMEM;
> ret = SendMessage( ctx->hwnd, WM_CAP_GET_VIDEOFORMAT, ret, (LPARAM) bih );
> if( !ret )
> return AVERROR_IO;
Need to free bih in case of error here.
> dump_bih( s, bih );
>
> codec = st->codec;
> codec->time_base = ap->time_base;
> codec->codec_type = CODEC_TYPE_VIDEO;
> codec->width = bih->bmiHeader.biWidth;
> codec->height = bih->bmiHeader.biHeight;
> codec->codec_id = CODEC_ID_RAWVIDEO;
> codec->pix_fmt = vfw_pixfmt( bih->bmiHeader.biCompression );
>
> av_set_pts_info( st, 32, 1, 1000 );
>
> av_free( bih );
>
> return 0;
> }
>
> static int vfw_read_packet( AVFormatContext *s, AVPacket *pkt )
> {
> struct vfw_ctx *ctx = s->priv_data;
> int ret;
>
> ctx->pkt = pkt;
> ctx->grabbed = 0;
>
> ret = SendMessage( ctx->hwnd, WM_CAP_GRAB_FRAME, 0, 0 );
> if( !ret || !ctx->grabbed )
> return AVERROR_IO;
>
> return pkt->size;
> }
>
> static int vfw_read_close( AVFormatContext *s )
> {
> struct vfw_ctx *ctx = s->priv_data;
>
> SendMessage( ctx->hwnd, WM_CAP_SET_CALLBACK_FRAME, 0, 0 );
> SendMessage( ctx->hwnd, WM_CAP_DRIVER_DISCONNECT, 0, 0 );
> DestroyWindow( ctx->hwnd );
>
> return 0;
> }
>
> AVInputFormat vfwcap_demuxer = {
> "vfwcap",
> "VFW video capture",
> sizeof(struct vfw_ctx),
> NULL,
> vfw_read_header,
> vfw_read_packet,
> vfw_read_close,
> .flags = AVFMT_NOFILE,
> };
>
--
M?ns Rullg?rd
mans at mansr.com
More information about the ffmpeg-devel
mailing list