[FFmpeg-devel] Fw: [PATCH] DirectShow patches
Ramiro Polla
ramiro.polla at gmail.com
Thu Sep 8 02:34:58 CEST 2011
Hi,
Thanks for reviewing/applying.
>> > From 7f31d80dcb137fdd15bfef855e4dabe6aca9f527 Mon Sep 17 00:00:00 2001
>> > From: Ramiro Polla <ramiro.polla at gmail.com>
>> > Date: Fri, 2 Sep 2011 01:33:41 -0300
>> > Subject: [PATCH 3/7] dshow: add option to list devices
>> >
>> > ---
>> > libavdevice/dshow.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>> > 1 files changed, 42 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
>> > index 9913569..555a2b8 100644
>> > --- a/libavdevice/dshow.c
>> > +++ b/libavdevice/dshow.c
[...]
>> > @@ -263,6 +278,9 @@ fail1:
>> > IPropertyBag_Release(bag);
>> > IMoniker_Release(m);
>> > }
>>
>> > + if (list_devices) {
>> > + goto error;
>> > + }
>>
>> And set ret to AVERROR_EXIT, like it is done in the openal input
>> device.
Fixed.
>> > From 2cb98e2cd3b29bc49d9dedd7af6bc3a10beadf3e Mon Sep 17 00:00:00 2001
>> > From: Ramiro Polla <ramiro.polla at gmail.com>
>> > Date: Fri, 2 Sep 2011 01:33:57 -0300
>> > Subject: [PATCH 4/7] dshow: add audio/video options
>> >
>> > ---
>> > libavdevice/dshow.c | 150 ++++++++++++++++++++++++++++++++++++++++++++
>> > libavdevice/dshow.h | 2 +
>> > libavdevice/dshow_common.c | 49 ++++++++++++++
>> > 3 files changed, 201 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
>> > index 555a2b8..4cda218 100644
>> > --- a/libavdevice/dshow.c
>> > +++ b/libavdevice/dshow.c
[...]
>> > @@ -210,6 +222,109 @@ fail:
>> > }
>> >
>> > static int
>> > +dshow_set_format(AVFormatContext *avctx, enum dshowDeviceType devtype,
>> > + IPin *pin, AM_MEDIA_TYPE *type)
>> > +{
>> > + struct dshow_ctx *ctx = avctx->priv_data;
>> > + IAMStreamConfig *c = NULL;
>>
>> nit: better to call it "config", more readable
Ok.
>> > + void *caps = NULL;
>> > + int i, n, size;
>> > + int ret = 0;
>> > +
>> > + if (IPin_QueryInterface(pin, &IID_IAMStreamConfig, (void **) &c) != S_OK)
>> > + return 0;
>> > + if (IAMStreamConfig_GetNumberOfCapabilities(c, &n, &size) != S_OK)
>> > + goto end;
>> > +
>> > + caps = av_malloc(size);
>> > + if (!caps)
>> > + goto end;
>> > +
>> > + for (i = 0; i < n; i++) {
>> > + IAMStreamConfig_GetStreamCaps(c, i, &type, (void *) caps);
>> > +
>> > +#if DSHOWDEBUG
>> > + ff_print_AM_MEDIA_TYPE(type);
>> > +#endif
>> > +
>> > + if (devtype == VideoDevice) {
>> > + VIDEO_STREAM_CONFIG_CAPS *vcaps = caps;
>> > + BITMAPINFOHEADER *bih;
>> > + int64_t *fr;
>> > +#if DSHOWDEBUG
>> > + ff_print_VIDEO_STREAM_CONFIG_CAPS(vcaps);
>> > +#endif
>> > + if (IsEqualGUID(&type->formattype, &FORMAT_VideoInfo)) {
>> > + VIDEOINFOHEADER *v = (void *) type->pbFormat;
>>
>> nit++: maybe ih/ih2 for consistency with the code below (fx).
It's already like this in another part of the code. I don't mind if
you change them all consistently though.
>> > + fr = &v->AvgTimePerFrame;
>> > + bih = &v->bmiHeader;
>> > + } else if (IsEqualGUID(&type->formattype, &FORMAT_VideoInfo2)) {
>> > + VIDEOINFOHEADER2 *v = (void *) type->pbFormat;
>>
>> > + fr = &v->AvgTimePerFrame;
>> > + bih = &v->bmiHeader;
>> > + } else {
>> > + continue;
>> > + }
>> > + if (ctx->framerate) {
>>
>> > + int64_t framerate = (ctx->requested_framerate.den*10000000)
>> > + / ctx->requested_framerate.num;
>>
>> possible overflow? A cast to int64_t should be enough to fix it.
Fixed.
>> > + if (framerate > vcaps->MaxFrameInterval ||
>> > + framerate < vcaps->MinFrameInterval)
>> > + continue;
>> > + *fr = framerate;
>> > + }
>> > + if (ctx->video_size) {
>> > + if (ctx->requested_width > vcaps->MaxOutputSize.cx ||
>> > + ctx->requested_width < vcaps->MinOutputSize.cx ||
>>
>> > + ctx->requested_height < vcaps->MinOutputSize.cy ||
>> > + ctx->requested_height < vcaps->MinOutputSize.cy)
>>
>> typo: missing > vcaps->MaxOutputSize.cy check
Fixed, thanks for spotting.
>> > @@ -656,6 +801,11 @@ static int dshow_read_packet(AVFormatContext *s, AVPacket *pkt)
>> > #define OFFSET(x) offsetof(struct dshow_ctx, x)
>> > #define DEC AV_OPT_FLAG_DECODING_PARAM
>> > static const AVOption options[] = {
>>
>> > + { "video_size", "A string describing frame size, such as 640x480 or hd720.", OFFSET(video_size), FF_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
>> > + { "framerate", "", OFFSET(framerate), FF_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
>> > + { "sample_rate", "", OFFSET(sample_rate), FF_OPT_TYPE_INT, {.dbl = 0}, 0, INT_MAX, DEC },
>> > + { "sample_size", "", OFFSET(sample_size), FF_OPT_TYPE_INT, {.dbl = 0}, 0, 16, DEC },
>> > + { "channels", "", OFFSET(channels), FF_OPT_TYPE_INT, {.dbl = 0}, 0, INT_MAX, DEC },
>>
>> please document all these, also semantics is "what the option does"
>> rather than "what the argument is", so documentation for video_size
>> should be:
>> "set frame size, accept a string as ..." - no need for the ending point
I just copied the first from some other place. The others I thought
were straight-forward enough to omit. I've added something in the new
patches, but if feel free to change to whatever you think is best.
>> > From bdb1c350d63f3a29b187405d33e45ddade6d287f Mon Sep 17 00:00:00 2001
>> > From: Ramiro Polla <ramiro.polla at gmail.com>
>> > Date: Fri, 2 Sep 2011 01:34:43 -0300
>> > Subject: [PATCH 5/7] dshow: add option to list audio/video options
>> >
>> > ---
>> > libavdevice/dshow.c | 37 ++++++++++++++++++++++++++++++++++---
>> > 1 files changed, 34 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
>> > index 4cda218..133e7be 100644
>> > --- a/libavdevice/dshow.c
>> > +++ b/libavdevice/dshow.c
[...]
>> > @@ -265,6 +266,14 @@ dshow_set_format(AVFormatContext *avctx, enum dshowDeviceType devtype,
>> > } else {
>> > continue;
>> > }
>> > + if (list_options) {
>> > + av_log(avctx, AV_LOG_INFO, " %ldx%ld %gfps -> %ldx%ld %gfps\n",
>>
>> nit maybe: min_size=WxH min_fps=%f max_size=WxH min_fps=...
>>
>> so you don't need to check the code to understand what the "->" means
>> in this context
Changed a bit.
>> > @@ -809,6 +837,9 @@ static const AVOption options[] = {
>> > { "list_devices", "list available devices", OFFSET(list_devices), FF_OPT_TYPE_INT, {.dbl=0}, 0, 1, DEC, "list_devices" },
>> > { "true", "", 0, FF_OPT_TYPE_CONST, {.dbl=1}, 0, 0, DEC, "list_devices" },
>> > { "false", "", 0, FF_OPT_TYPE_CONST, {.dbl=0}, 0, 0, DEC, "list_devices" },
>> > + { "list_options", "list available options for specified device", OFFSET(list_options), FF_OPT_TYPE_INT, {.dbl=0}, 0, 1, DEC, "list_options" },
>> > + { "true", "", 0, FF_OPT_TYPE_CONST, {.dbl=1}, 0, 0, DEC, "list_options" },
>> > + { "false", "", 0, FF_OPT_TYPE_CONST, {.dbl=0}, 0, 0, DEC, "list_options" },
>> > { NULL },
>> > };
>>
>> Would be possible to have distinct list_options() and list_devices()
>> functions, rather than have their code interspersed with the
>> set_format()/open_device() code?
>>
>> I'm fine to do that with another patch if possible, should help
>> readability/maintainability.
I've factorised the code a bit to allow this. I think it's better now.
>> > From 187fb0f71a283e929712b8d47b7f540ee89eebc1 Mon Sep 17 00:00:00 2001
>> > From: Ramiro Polla <ramiro.polla at gmail.com>
>> > Date: Fri, 2 Sep 2011 01:35:21 -0300
>> > Subject: [PATCH 6/7] doc: add documentation for dshow indev
>> >
>> > ---
>> > doc/indevs.texi | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> > 1 files changed, 73 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/doc/indevs.texi b/doc/indevs.texi
>> > index c38031f..af95e00 100644
>> > --- a/doc/indevs.texi
>> > +++ b/doc/indevs.texi
>> > @@ -55,6 +55,79 @@ For more information see:
>> >
>> > BSD video input device.
>> >
>> > + at section dshow
>> > +
>> > +Windows DirectShow input device.
>> > +
>> > +DirectShow support is enabled when FFmpeg is built with mingw-w64.
>> > +Currently only audio and video devices are supported.
>> > +
>> > +Multiple devices may be opened as separate inputs, but they may also be
>> > +opened on the same input, which should improve synchronism between them.
>> > +
>> > +The input name should be in the format:
>> > +
>> > + at example
>> > + at var{TYPE}=@var{NAME}[:@var{TYPE}=@var{NAME}]
>> > + at end example
>> > +
>> > +where @var{TYPE} can be either @var{audio} or @var{video},
>> > +and @var{NAME} is the device's name.
>> > +
>> > + at subsection Options
>> > +
>> > +If no options are specified, the device's defaults are used.
>>
>> > +If the device does not support the requested options, FFmpeg
>> > +will fail to open it.
>>
>> Not clear what "FFmpeg" means in this case (the tool?, the project
>> code?), maybe better:
>>
>> If the accessed DirectShow device does not support the requested
>> options, it will fail to open it.
Double "it"s are confusing too. I've changed it to something else.
>> > +
>> > + at table @option
>> > +
>> > + at item video_size
>> > +Set the video size in the captured video.
>> > +
>> > + at item framerate
>> > +Set the framerate in the captured video.
>> > +
>> > + at item sample_rate
>> > +Set the sample rate (in Hz) of the captured audio.
>> > +
>> > + at item sample_size
>> > +Set the sample size (in bits) of the captured audio.
>> > +
>> > + at item channels
>> > +Set the number of channels in the captured audio.
>> > +
>> > + at item list_devices
>> > +If set to @option{true}, print a list of devices and exit.
>> > +
>> > + at item list_options
>> > +If set to @option{true}, print a list of selected device's options
>> > +and exit.
>> > +
>> > + at end table
>> > +
>>
>> > + at subsection Examples
>> > +
>> > +Print the list of DirectShow supported devices and exit:
>> > + at example
>> > +$ ffmpeg -list_devices true -f dshow -i dummy
>> > + at end example
>> > +
>> > +Open video device @var{Camera}:
>> > + at example
>> > +$ ffmpeg -f dshow -i video="Camera"
>> > + at end example
>> > +
>> > +Open video device @var{Camera} and audio device @var{Microphone}:
>> > + at example
>> > +$ ffmpeg -f dshow -i video="Camera":audio="Microphone"
>> > + at end example
>> > +
>> > +Print the list of supported options in selected device and exit:
>> > + at example
>> > +$ ffmpeg -list_options true -f dshow -i video="Camera"
>> > + at end example
>> > +
>>
>> If you want to increase the niceness level:
>> @itemize
>> @item Do this...
>> @example
>> ...
>> @end example
>>
>> like in the lavfi device docs.
Done.
New patches attached.
Ramiro
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-dshow-factorise-cycling-through-devices.patch
Type: text/x-patch
Size: 3481 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-dshow-add-option-to-list-devices.patch
Type: text/x-patch
Size: 3597 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-dshow-indent.patch
Type: text/x-patch
Size: 1524 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-dshow-factorise-cycling-through-pins.patch
Type: text/x-patch
Size: 3517 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-dshow-initialize-variable-to-prevent-releasing-rando.patch
Type: text/x-patch
Size: 828 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-dshow-add-audio-video-options.patch
Type: text/x-patch
Size: 12196 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-dshow-add-option-to-list-audio-video-options.patch
Type: text/x-patch
Size: 5292 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-dshow-indent.patch
Type: text/x-patch
Size: 1478 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-doc-add-documentation-for-dshow-indev.patch
Type: text/x-patch
Size: 2469 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110907/ad07e3b4/attachment-0008.bin>
More information about the ffmpeg-devel
mailing list