[FFmpeg-devel] Fw: [PATCH] DirectShow patches

Stefano Sabatini stefano.sabatini-lala at poste.it
Thu Sep 8 16:55:03 CEST 2011


On date Wednesday 2011-09-07 21:34:58 -0300, Ramiro Polla encoded:
> Hi,
> 
> Thanks for reviewing/applying.
[...] 
> Done.
> 
> New patches attached.
> 
> Ramiro

> From 72bf72ce30c09dff4977ef3b4f12f80caff931e4 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:09:31 -0300
> Subject: [PATCH 1/9] dshow: factorise cycling through devices
> 
> ---
>  libavdevice/dshow.c |   48 ++++++++++++++++++++++++++++++++++--------------
>  1 files changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 10e3d4f..aa29b6b 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -214,35 +214,26 @@ fail:
>  }
>  
>  static int
> -dshow_open_device(AVFormatContext *avctx, ICreateDevEnum *devenum,
> -                  enum dshowDeviceType devtype)
> +dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum,
> +                    enum dshowDeviceType devtype, IBaseFilter **pfilter)

Please add a short doxy, it is not that easy to understand what the
code does.

>From my understanding:

|Cycle through available devices using the device enumerator devenum,
|retrieve the one with type specified by devtype and return the
|pointer to the found object in *pfilter.

>  {
>      struct dshow_ctx *ctx = avctx->priv_data;
>      IBaseFilter *device_filter = NULL;
>      IEnumMoniker *classenum = NULL;
> -    IGraphBuilder *graph = ctx->graph;
> -    IEnumPins *pins = 0;
>      IMoniker *m = NULL;
> -    IPin *device_pin = NULL;
> -    libAVPin *capture_pin = NULL;
> -    libAVFilter *capture_filter = NULL;
>      const char *device_name = ctx->device_name[devtype];
> -    int ret = AVERROR(EIO);
> -    IPin *pin;
>      int r;
>  
>      const GUID *device_guid[2] = { &CLSID_VideoInputDeviceCategory,
>                                     &CLSID_AudioInputDeviceCategory };
> -    const GUID *mediatype[2] = { &MEDIATYPE_Video, &MEDIATYPE_Audio };
>      const char *devtypename = (devtype == VideoDevice) ? "video" : "audio";
> -    const wchar_t *filter_name[2] = { L"Audio capture filter", L"Video capture filter" };
>  
>      r = ICreateDevEnum_CreateClassEnumerator(devenum, device_guid[devtype],
>                                               (IEnumMoniker **) &classenum, 0);
>      if (r != S_OK) {
>          av_log(avctx, AV_LOG_ERROR, "Could not enumerate %s devices.\n",
>                 devtypename);
> -        goto error;
> +        return AVERROR(EIO);
>      }
>  
>      while (IEnumMoniker_Next(classenum, 1, &m, NULL) == S_OK && !device_filter) {
> @@ -274,11 +265,42 @@ fail1:
>          IMoniker_Release(m);
>      }
>  
> +    IEnumMoniker_Release(classenum);
> +
>      if (!device_filter) {
>          av_log(avctx, AV_LOG_ERROR, "Could not find %s device.\n",
>                 devtypename);
> +        return AVERROR(EIO);
> +    }
> +    *pfilter = device_filter;
> +
> +    return 0;
> +}
> +
> +static int
> +dshow_open_device(AVFormatContext *avctx, ICreateDevEnum *devenum,
> +                  enum dshowDeviceType devtype)
> +{
> +    struct dshow_ctx *ctx = avctx->priv_data;
> +    IBaseFilter *device_filter = NULL;
> +    IGraphBuilder *graph = ctx->graph;
> +    IEnumPins *pins = 0;
> +    IPin *device_pin = NULL;
> +    libAVPin *capture_pin = NULL;
> +    libAVFilter *capture_filter = NULL;
> +    int ret = AVERROR(EIO);
> +    IPin *pin;
> +    int r;
> +
> +    const GUID *mediatype[2] = { &MEDIATYPE_Video, &MEDIATYPE_Audio };
> +    const char *devtypename = (devtype == VideoDevice) ? "video" : "audio";
> +    const wchar_t *filter_name[2] = { L"Audio capture filter", L"Video capture filter" };
> +
> +    if ((r = dshow_cycle_devices(avctx, devenum, devtype, &device_filter)) < 0) {
> +        ret = r;
>          goto error;
>      }
>      ctx->device_filter [devtype] = device_filter;
>  
>      r = IGraphBuilder_AddFilter(graph, device_filter, NULL);
> @@ -371,8 +393,6 @@ next:
>  error:
>      if (pins)
>          IEnumPins_Release(pins);
> -    if (classenum)
> -        IEnumMoniker_Release(classenum);
>  
>      return ret;
>  }

Looks nice otherwise.

> From 142f9aaa7c1511b769d5ebbba16279cba164f987 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:09:41 -0300
> Subject: [PATCH 2/9] dshow: add option to list devices
> 
> ---
>  libavdevice/dshow.c |   40 +++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 39 insertions(+), 1 deletions(-)
> 
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index aa29b6b..45137d5 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -19,14 +19,20 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> +#include "libavutil/opt.h"
> +
>  #include "avdevice.h"
>  #include "dshow.h"
>  
>  struct dshow_ctx {
> +    AVClass *class;

const?

> +
>      IGraphBuilder *graph;
>  
>      char *device_name[2];
>  
> +    int   list_devices;
> +
>      IBaseFilter *device_filter[2];
>      IPin        *device_pin[2];
>      libAVFilter *capture_filter[2];
> @@ -252,10 +258,14 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum,
>  
>          buf = dup_wchar_to_utf8(var.bstrVal);
>  
> +        if (pfilter) {
>          if (strcmp(device_name, buf))
>              goto fail1;
>  
>          IMoniker_BindToObject(m, 0, 0, &IID_IBaseFilter, (void *) &device_filter);
> +        } else {
> +            av_log(avctx, AV_LOG_INFO, " \"%s\"\n", buf);
> +        }
>  
>  fail1:
>          if (buf)
> @@ -267,12 +277,14 @@ fail1:
>  
>      IEnumMoniker_Release(classenum);
>  
> +    if (pfilter) {
>      if (!device_filter) {
>          av_log(avctx, AV_LOG_ERROR, "Could not find %s device.\n",
>                 devtypename);
>          return AVERROR(EIO);
>      }
>      *pfilter = device_filter;
> +    }
>  
>      return 0;
>  }

I'm not very fond of using the pfilter param for forcing an operation
with different semantics (looking for a device/listing all the
devices), but I have no suggestions for doing it better, so I won't
object.

> @@ -550,7 +562,7 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
>      int ret = AVERROR(EIO);
>      int r;
>  
> -    if (!parse_device_name(avctx)) {
> +    if (!ctx->list_devices && !parse_device_name(avctx)) {
>          av_log(avctx, AV_LOG_ERROR, "Malformed dshow input string.\n");
>          goto error;
>      }
> @@ -572,6 +584,15 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
>          goto error;
>      }
>  
> +    if (ctx->list_devices) {
> +        av_log(avctx, AV_LOG_INFO, "DirectShow video devices\n");
> +        dshow_cycle_devices(avctx, devenum, VideoDevice, NULL);
> +        av_log(avctx, AV_LOG_INFO, "DirectShow audio devices\n");
> +        dshow_cycle_devices(avctx, devenum, AudioDevice, NULL);
> +        ret = AVERROR_EXIT;
> +        goto error;
> +    }
> +
>      if (ctx->device_name[VideoDevice]) {
>          ret = dshow_open_device(avctx, devenum, VideoDevice);
>          if (ret < 0)
> @@ -659,6 +680,22 @@ static int dshow_read_packet(AVFormatContext *s, AVPacket *pkt)
>      return pkt->size;
>  }
>  
> +#define OFFSET(x) offsetof(struct dshow_ctx, x)
> +#define DEC AV_OPT_FLAG_DECODING_PARAM
> +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" },
> +    { NULL },
> +};
> +
> +static const AVClass dshow_class = {
> +    .class_name = "DirectShow indev",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
>  AVInputFormat ff_dshow_demuxer = {
>      "dshow",
>      NULL_IF_CONFIG_SMALL("DirectShow capture"),
> @@ -668,4 +705,5 @@ AVInputFormat ff_dshow_demuxer = {
>      dshow_read_packet,
>      dshow_read_close,
>      .flags = AVFMT_NOFILE,
> +    .priv_class = &dshow_class,
>  };
> -- 
> 1.7.4.1

Looks fine otherwise.

> From f9d0c51b09b40414f789ebd990a59a875518f5d4 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:09:48 -0300
> Subject: [PATCH 3/9] dshow: indent
> 
> ---
>  libavdevice/dshow.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
[...] 

> From 4316bf93d2afd342b475b54c4636a0df75125e95 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:10:23 -0300
> Subject: [PATCH 4/9] dshow: factorise cycling through pins
> 
> ---
>  libavdevice/dshow.c |   68 +++++++++++++++++++++++++++++++-------------------
>  1 files changed, 42 insertions(+), 26 deletions(-)
> 
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index fdf63f2..455603b 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -290,41 +290,21 @@ fail1:
>  }
>  
>  static int
> -dshow_open_device(AVFormatContext *avctx, ICreateDevEnum *devenum,
> -                  enum dshowDeviceType devtype)
> +dshow_cycle_pins(AVFormatContext *avctx, enum dshowDeviceType devtype,
> +                 IBaseFilter *device_filter, IPin **ppin)

Same here, a doxy may help, I don't want to force you on that though.

>  {
> -    struct dshow_ctx *ctx = avctx->priv_data;
> -    IBaseFilter *device_filter = NULL;
> -    IGraphBuilder *graph = ctx->graph;
>      IEnumPins *pins = 0;
>      IPin *device_pin = NULL;
> -    libAVPin *capture_pin = NULL;
> -    libAVFilter *capture_filter = NULL;
> -    int ret = AVERROR(EIO);
>      IPin *pin;
>      int r;
>  
>      const GUID *mediatype[2] = { &MEDIATYPE_Video, &MEDIATYPE_Audio };
>      const char *devtypename = (devtype == VideoDevice) ? "video" : "audio";
> -    const wchar_t *filter_name[2] = { L"Audio capture filter", L"Video capture filter" };
> -
> -    if ((r = dshow_cycle_devices(avctx, devenum, devtype, &device_filter)) < 0) {
> -        ret = r;
> -        goto error;
> -    }
> -
> -    ctx->device_filter [devtype] = device_filter;
> -
> -    r = IGraphBuilder_AddFilter(graph, device_filter, NULL);
> -    if (r != S_OK) {
> -        av_log(avctx, AV_LOG_ERROR, "Could not add device filter to graph.\n");
> -        goto error;
> -    }
>  
>      r = IBaseFilter_EnumPins(device_filter, &pins);
>      if (r != S_OK) {
>          av_log(avctx, AV_LOG_ERROR, "Could not enumerate pins.\n");
> -        goto error;
> +        return AVERROR(EIO);
>      }
>  
>      while (IEnumPins_Next(pins, 1, &pin, NULL) == S_OK && !device_pin) {
> @@ -369,9 +349,48 @@ next:
>              IPin_Release(pin);
>      }
>  
> +    IEnumPins_Release(pins);
> +
>      if (!device_pin) {
>          av_log(avctx, AV_LOG_ERROR,
>                 "Could not find output pin from %s capture device.\n", devtypename);
> +        return AVERROR(EIO);
> +    }
> +    *ppin = device_pin;
> +
> +    return 0;
> +}
> +
> +static int
> +dshow_open_device(AVFormatContext *avctx, ICreateDevEnum *devenum,
> +                  enum dshowDeviceType devtype)
> +{
> +    struct dshow_ctx *ctx = avctx->priv_data;
> +    IBaseFilter *device_filter = NULL;
> +    IGraphBuilder *graph = ctx->graph;
> +    IPin *device_pin = NULL;
> +    libAVPin *capture_pin = NULL;
> +    libAVFilter *capture_filter = NULL;
> +    int ret = AVERROR(EIO);
> +    int r;
> +
> +    const wchar_t *filter_name[2] = { L"Audio capture filter", L"Video capture filter" };
> +
> +    if ((r = dshow_cycle_devices(avctx, devenum, devtype, &device_filter)) < 0) {
> +        ret = r;
> +        goto error;
> +    }
> +
> +    ctx->device_filter [devtype] = device_filter;
> +
> +    r = IGraphBuilder_AddFilter(graph, device_filter, NULL);
> +    if (r != S_OK) {
> +        av_log(avctx, AV_LOG_ERROR, "Could not add device filter to graph.\n");
> +        goto error;
> +    }
> +
> +    if ((r = dshow_cycle_pins(avctx, devtype, device_filter, &device_pin)) < 0) {
> +        ret = r;
>          goto error;
>      }
>      ctx->device_pin[devtype] = device_pin;
> @@ -403,9 +422,6 @@ next:
>      ret = 0;
>  
>  error:
> -    if (pins)
> -        IEnumPins_Release(pins);
> -
>      return ret;
>  }
>  
> -- 
> 1.7.4.1

Sounds good.


> From 79720cd03fc0e629beaadc1337315a8d8a566be0 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:11:18 -0300
> Subject: [PATCH 5/9] dshow: initialize variable to prevent releasing random data
> 
> ---
>  libavdevice/dshow.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 455603b..0d8042c 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -309,7 +309,7 @@ dshow_cycle_pins(AVFormatContext *avctx, enum dshowDeviceType devtype,
>  
>      while (IEnumPins_Next(pins, 1, &pin, NULL) == S_OK && !device_pin) {
>          IKsPropertySet *p = NULL;
> -        IEnumMediaTypes *types;
> +        IEnumMediaTypes *types = NULL;
>          PIN_INFO info = {0};
>          AM_MEDIA_TYPE *type;
>          GUID category;
> -- 
> 1.7.4.1

Looks safe.

> From 203b67b6e3c7348e973772ccc7ea30d3ab679e0e Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:11:32 -0300
> Subject: [PATCH 6/9] dshow: add audio/video options
> 
> ---
>  libavdevice/dshow.c        |  154 ++++++++++++++++++++++++++++++++++++++++++++
>  libavdevice/dshow.h        |    2 +
>  libavdevice/dshow_common.c |   49 ++++++++++++++
>  3 files changed, 205 insertions(+), 0 deletions(-)
> 
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 0d8042c..27bbeaf 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -19,6 +19,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> +#include "libavutil/parseutils.h"
>  #include "libavutil/opt.h"
>  
>  #include "avdevice.h"
> @@ -46,6 +47,17 @@ struct dshow_ctx {
>      unsigned int video_frame_num;
>  
>      IMediaControl *control;
> +
> +    char *video_size;
> +    char *framerate;
> +
> +    int requested_width;
> +    int requested_height;
> +    AVRational requested_framerate;
> +
> +    int sample_rate;
> +    int sample_size;
> +    int channels;
>  };
>  
>  static enum PixelFormat dshow_pixfmt(DWORD biCompression, WORD biBitCount)
> @@ -289,10 +301,117 @@ fail1:
>      return 0;
>  }
>  
> +static void
> +dshow_cycle_formats(AVFormatContext *avctx, enum dshowDeviceType devtype,
> +                    IPin *pin, AM_MEDIA_TYPE *type, int *pset)

Again: a doxy may help here, especially given the funny names MS devs
like to use for their APIs.

Nit: pset -> pformat_set should be more clear/less ambiguous

> +{
> +    struct dshow_ctx *ctx = avctx->priv_data;
> +    IAMStreamConfig *config = NULL;
> +    int format_set = 0;
> +    void *caps = NULL;
> +    int i, n, size;
> +
> +    if (IPin_QueryInterface(pin, &IID_IAMStreamConfig, (void **) &config) != S_OK)
> +        return;
> +    if (IAMStreamConfig_GetNumberOfCapabilities(config, &n, &size) != S_OK)
> +        goto end;
> +
> +    caps = av_malloc(size);
> +    if (!caps)
> +        goto end;
> +
> +    for (i = 0; i < n && !format_set; i++) {
> +        IAMStreamConfig_GetStreamCaps(config, 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;
> +                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 {
> +                goto next;
> +            }
> +            if (ctx->framerate) {
> +                int64_t framerate = ((int64_t) ctx->requested_framerate.den*10000000)
> +                                            /  ctx->requested_framerate.num;
> +                if (framerate > vcaps->MaxFrameInterval ||
> +                    framerate < vcaps->MinFrameInterval)
> +                    goto next;
> +                *fr = framerate;
> +            }
> +            if (ctx->video_size) {
> +                if (ctx->requested_width  > vcaps->MaxOutputSize.cx ||
> +                    ctx->requested_width  < vcaps->MinOutputSize.cx ||
> +                    ctx->requested_height > vcaps->MaxOutputSize.cy ||
> +                    ctx->requested_height < vcaps->MinOutputSize.cy)
> +                    goto next;
> +                bih->biWidth  = ctx->requested_width;
> +                bih->biHeight = ctx->requested_height;
> +            }
> +        } else {
> +            AUDIO_STREAM_CONFIG_CAPS *acaps = caps;
> +            WAVEFORMATEX *fx;
> +#if DSHOWDEBUG
> +            ff_print_AUDIO_STREAM_CONFIG_CAPS(acaps);
> +#endif
> +            if (IsEqualGUID(&type->formattype, &FORMAT_WaveFormatEx)) {
> +                fx = (void *) type->pbFormat;
> +            } else {
> +                goto next;
> +            }
> +            if (ctx->sample_rate) {
> +                if (ctx->sample_rate > acaps->MaximumSampleFrequency ||
> +                    ctx->sample_rate < acaps->MinimumSampleFrequency)
> +                    goto next;
> +                fx->nSamplesPerSec = ctx->sample_rate;
> +            }
> +            if (ctx->sample_size) {
> +                if (ctx->sample_size > acaps->MaximumBitsPerSample ||
> +                    ctx->sample_size < acaps->MinimumBitsPerSample)
> +                    goto next;
> +                fx->wBitsPerSample = ctx->sample_size;
> +            }
> +            if (ctx->channels) {
> +                if (ctx->channels > acaps->MaximumChannels ||
> +                    ctx->channels < acaps->MinimumChannels)
> +                    goto next;
> +                fx->nChannels = ctx->channels;
> +            }
> +        }
> +        if (IAMStreamConfig_SetFormat(config, type) != S_OK)
> +            goto next;
> +        format_set = 1;
> +next:
> +        if (type->pbFormat)
> +            CoTaskMemFree(type->pbFormat);
> +        CoTaskMemFree(type);
> +    }
> +end:
> +    IAMStreamConfig_Release(config);
> +    if (caps)
> +        av_free(caps);
> +    *pset = format_set;
> +}
> +
>  static int
>  dshow_cycle_pins(AVFormatContext *avctx, enum dshowDeviceType devtype,
>                   IBaseFilter *device_filter, IPin **ppin)
>  {
> +    struct dshow_ctx *ctx = avctx->priv_data;
>      IEnumPins *pins = 0;
>      IPin *device_pin = NULL;
>      IPin *pin;
> @@ -301,6 +420,10 @@ dshow_cycle_pins(AVFormatContext *avctx, enum dshowDeviceType devtype,
>      const GUID *mediatype[2] = { &MEDIATYPE_Video, &MEDIATYPE_Audio };
>      const char *devtypename = (devtype == VideoDevice) ? "video" : "audio";
>  
> +    int set_format = (devtype == VideoDevice && (ctx->video_size || ctx->framerate))
> +                  || (devtype == AudioDevice && (ctx->channels || ctx->sample_rate));
> +    int format_set = !set_format;

ugh, this is confusing, I'd prefer to keep format_set to 0.

> +
>      r = IBaseFilter_EnumPins(device_filter, &pins);
>      if (r != S_OK) {
>          av_log(avctx, AV_LOG_ERROR, "Could not enumerate pins.\n");
> @@ -328,6 +451,13 @@ dshow_cycle_pins(AVFormatContext *avctx, enum dshowDeviceType devtype,
>          if (!IsEqualGUID(&category, &PIN_CATEGORY_CAPTURE))
>              goto next;
>  
> +        if (set_format) {
> +            dshow_cycle_formats(avctx, devtype, pin, type, &format_set);
> +            if (!format_set) {
> +                goto next;
> +            }
> +        }
> +
>          if (IPin_EnumMediaTypes(pin, &types) != S_OK)
>              goto next;
>  
> @@ -351,6 +481,10 @@ next:
>  
>      IEnumPins_Release(pins);
>  

> +    if (!format_set) {
> +        av_log(avctx, AV_LOG_ERROR, "Could not set %s options\n", devtypename);
> +        return AVERROR(EIO);
> +    }

and change the check here to:
if (set_format && !format_set) // it was requested to set format, but no format could be set

>      if (!device_pin) {
>          av_log(avctx, AV_LOG_ERROR,
>                 "Could not find output pin from %s capture device.\n", devtypename);
> @@ -583,6 +717,21 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
>          goto error;
>      }
>  
> +    if (ctx->video_size) {
> +        r = av_parse_video_size(&ctx->requested_width, &ctx->requested_height, ctx->video_size);
> +        if (r < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Couldn't parse video size.\n");
> +            goto error;
> +        }
> +    }
> +    if (ctx->framerate) {
> +        r = av_parse_video_rate(&ctx->requested_framerate, ctx->framerate);
> +        if (r < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Could not parse framerate '%s'.\n", ctx->framerate);
> +            goto error;
> +        }
> +    }
> +
>      CoInitialize(0);
>  
>      r = CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER,
> @@ -699,6 +848,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 },

Nit: "set video size given a string such as  ..."

> +    { "framerate", "set video frame rate", OFFSET(framerate), FF_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> +    { "sample_rate", "set audio sample rate, such as 8000 or 44100", OFFSET(sample_rate), FF_OPT_TYPE_INT, {.dbl = 0}, 0, INT_MAX, DEC },
> +    { "sample_size", "set audio sample size, such as 8 or 16", OFFSET(sample_size), FF_OPT_TYPE_INT, {.dbl = 0}, 0, 16, DEC },
> +    { "channels", "set number of audio channels, such as 1 or 2", OFFSET(channels), FF_OPT_TYPE_INT, {.dbl = 0}, 0, INT_MAX, DEC },
>      { "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" },
> diff --git a/libavdevice/dshow.h b/libavdevice/dshow.h
> index 4e79680..83c71c4 100644
> --- a/libavdevice/dshow.h
> +++ b/libavdevice/dshow.h
> @@ -29,6 +29,8 @@
>  #include <dvdmedia.h>
>  
>  long ff_copy_dshow_media_type(AM_MEDIA_TYPE *dst, const AM_MEDIA_TYPE *src);
> +void ff_print_VIDEO_STREAM_CONFIG_CAPS(const VIDEO_STREAM_CONFIG_CAPS *caps);
> +void ff_print_AUDIO_STREAM_CONFIG_CAPS(const AUDIO_STREAM_CONFIG_CAPS *caps);
>  void ff_print_AM_MEDIA_TYPE(const AM_MEDIA_TYPE *type);
>  void ff_printGUID(const GUID *g);
>  
> diff --git a/libavdevice/dshow_common.c b/libavdevice/dshow_common.c
> index c813dc1..8fe2f77 100644
> --- a/libavdevice/dshow_common.c
> +++ b/libavdevice/dshow_common.c
> @@ -82,6 +82,55 @@ static void dump_bih(void *s, BITMAPINFOHEADER *bih)
>  }
>  #endif
>  
> +void ff_print_VIDEO_STREAM_CONFIG_CAPS(const VIDEO_STREAM_CONFIG_CAPS *caps)
> +{
> +#if DSHOWDEBUG
> +    dshowdebug(" VIDEO_STREAM_CONFIG_CAPS\n");
> +    dshowdebug("  guid\t");
> +    ff_printGUID(&caps->guid);
> +    dshowdebug("\n");
> +    dshowdebug("  VideoStandard\t%lu\n", caps->VideoStandard);
> +    dshowdebug("  InputSize %ld\t%ld\n", caps->InputSize.cx, caps->InputSize.cy);
> +    dshowdebug("  MinCroppingSize %ld\t%ld\n", caps->MinCroppingSize.cx, caps->MinCroppingSize.cy);
> +    dshowdebug("  MaxCroppingSize %ld\t%ld\n", caps->MaxCroppingSize.cx, caps->MaxCroppingSize.cy);
> +    dshowdebug("  CropGranularityX\t%d\n", caps->CropGranularityX);
> +    dshowdebug("  CropGranularityY\t%d\n", caps->CropGranularityY);
> +    dshowdebug("  CropAlignX\t%d\n", caps->CropAlignX);
> +    dshowdebug("  CropAlignY\t%d\n", caps->CropAlignY);
> +    dshowdebug("  MinOutputSize %ld\t%ld\n", caps->MinOutputSize.cx, caps->MinOutputSize.cy);
> +    dshowdebug("  MaxOutputSize %ld\t%ld\n", caps->MaxOutputSize.cx, caps->MaxOutputSize.cy);
> +    dshowdebug("  OutputGranularityX\t%d\n", caps->OutputGranularityX);
> +    dshowdebug("  OutputGranularityY\t%d\n", caps->OutputGranularityY);
> +    dshowdebug("  StretchTapsX\t%d\n", caps->StretchTapsX);
> +    dshowdebug("  StretchTapsY\t%d\n", caps->StretchTapsY);
> +    dshowdebug("  ShrinkTapsX\t%d\n", caps->ShrinkTapsX);
> +    dshowdebug("  ShrinkTapsY\t%d\n", caps->ShrinkTapsY);
> +    dshowdebug("  MinFrameInterval\t%"PRId64"\n", caps->MinFrameInterval);
> +    dshowdebug("  MaxFrameInterval\t%"PRId64"\n", caps->MaxFrameInterval);
> +    dshowdebug("  MinBitsPerSecond\t%ld\n", caps->MinBitsPerSecond);
> +    dshowdebug("  MaxBitsPerSecond\t%ld\n", caps->MaxBitsPerSecond);
> +#endif
> +}
> +
> +void ff_print_AUDIO_STREAM_CONFIG_CAPS(const AUDIO_STREAM_CONFIG_CAPS *caps)
> +{
> +#if DSHOWDEBUG
> +    dshowdebug(" AUDIO_STREAM_CONFIG_CAPS\n");
> +    dshowdebug("  guid\t");
> +    ff_printGUID(&caps->guid);
> +    dshowdebug("\n");
> +    dshowdebug("  MinimumChannels\t%lu\n", caps->MinimumChannels);
> +    dshowdebug("  MaximumChannels\t%lu\n", caps->MaximumChannels);
> +    dshowdebug("  ChannelsGranularity\t%lu\n", caps->ChannelsGranularity);
> +    dshowdebug("  MinimumBitsPerSample\t%lu\n", caps->MinimumBitsPerSample);
> +    dshowdebug("  MaximumBitsPerSample\t%lu\n", caps->MaximumBitsPerSample);
> +    dshowdebug("  BitsPerSampleGranularity\t%lu\n", caps->BitsPerSampleGranularity);
> +    dshowdebug("  MinimumSampleFrequency\t%lu\n", caps->MinimumSampleFrequency);
> +    dshowdebug("  MaximumSampleFrequency\t%lu\n", caps->MaximumSampleFrequency);
> +    dshowdebug("  SampleFrequencyGranularity\t%lu\n", caps->SampleFrequencyGranularity);
> +#endif
> +}
> +
>  void ff_print_AM_MEDIA_TYPE(const AM_MEDIA_TYPE *type)
>  {
>  #if DSHOWDEBUG
> -- 
> 1.7.4.1
> 

> From 0bec081602d583b915d88a831384db251869c137 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:14:59 -0300
> Subject: [PATCH 7/9] dshow: add option to list audio/video options
> 
> ---
>  libavdevice/dshow.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 55 insertions(+), 0 deletions(-)
> 
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 27bbeaf..ba08e31 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -32,6 +32,7 @@ struct dshow_ctx {
>  
>      char *device_name[2];
>  
> +    int   list_options;
>      int   list_devices;
>  
>      IBaseFilter *device_filter[2];
> @@ -345,6 +346,14 @@ dshow_cycle_formats(AVFormatContext *avctx, enum dshowDeviceType devtype,
>              } else {
>                  goto next;
>              }
> +            if (!pset) {
> +                av_log(avctx, AV_LOG_INFO, "  min %ldx%ld %gfps max %ldx%ld %gfps\n",

uhm... still a bit strange, I suggest:
min s=WxH fps=N max s=WxH fps=N

easier to parse, for both men and machines.

> +                       vcaps->MinOutputSize.cx, vcaps->MinOutputSize.cy,
> +                       1e7 / vcaps->MinFrameInterval,
> +                       vcaps->MaxOutputSize.cx, vcaps->MaxOutputSize.cy,
> +                       1e7 / vcaps->MaxFrameInterval);
> +                continue;
> +            }
>              if (ctx->framerate) {
>                  int64_t framerate = ((int64_t) ctx->requested_framerate.den*10000000)
>                                              /  ctx->requested_framerate.num;
> @@ -373,6 +382,12 @@ dshow_cycle_formats(AVFormatContext *avctx, enum dshowDeviceType devtype,
>              } else {
>                  goto next;
>              }

> +            if (!pset) {
> +                av_log(avctx, AV_LOG_INFO, "  min %luch %lu-bit %6luHz max %luch %lu-bit %6luHz\n",

Same here, I suggest:
   min ch=N bps=B rate=RHz max ch=N bps=B rate=RHz

feel free to keep this if you disagree, I can propose a patch later.

> +                       acaps->MinimumChannels, acaps->MinimumBitsPerSample, acaps->MinimumSampleFrequency,
> +                       acaps->MaximumChannels, acaps->MaximumBitsPerSample, acaps->MaximumSampleFrequency);
> +                continue;
> +            }
>              if (ctx->sample_rate) {
>                  if (ctx->sample_rate > acaps->MaximumSampleFrequency ||
>                      ctx->sample_rate < acaps->MinimumSampleFrequency)
> @@ -404,6 +419,7 @@ end:
>      IAMStreamConfig_Release(config);
>      if (caps)
>          av_free(caps);
> +    if (pset)
>      *pset = format_set;
>  }
>  
> @@ -430,6 +446,10 @@ dshow_cycle_pins(AVFormatContext *avctx, enum dshowDeviceType devtype,
>          return AVERROR(EIO);
>      }
>  
> +    if (!ppin) {
> +        av_log(avctx, AV_LOG_INFO, "DirectShow %s device options\n",
> +               devtypename);
> +    }
>      while (IEnumPins_Next(pins, 1, &pin, NULL) == S_OK && !device_pin) {
>          IKsPropertySet *p = NULL;
>          IEnumMediaTypes *types = NULL;
> @@ -451,6 +471,13 @@ dshow_cycle_pins(AVFormatContext *avctx, enum dshowDeviceType devtype,
>          if (!IsEqualGUID(&category, &PIN_CATEGORY_CAPTURE))
>              goto next;
>  
> +        if (!ppin) {
> +            char *buf = dup_wchar_to_utf8(info.achName);
> +            av_log(avctx, AV_LOG_INFO, " Pin \"%s\"\n", buf);
> +            av_free(buf);
> +            dshow_cycle_formats(avctx, devtype, pin, type, NULL);
> +            goto next;
> +        }
>          if (set_format) {
>              dshow_cycle_formats(avctx, devtype, pin, type, &format_set);
>              if (!format_set) {
> @@ -481,6 +508,7 @@ next:
>  
>      IEnumPins_Release(pins);
>  
> +    if (ppin) {
>      if (!format_set) {
>          av_log(avctx, AV_LOG_ERROR, "Could not set %s options\n", devtypename);
>          return AVERROR(EIO);
> @@ -491,6 +519,22 @@ next:
>          return AVERROR(EIO);
>      }
>      *ppin = device_pin;
> +    }
> +
> +    return 0;
> +}

In general I'm not very fond of this kind of code, indeed it took me a
bit of time to understand it, and I feel it can be made more clear by
creating specialized functions with no "hybrid" behavior depending on
the passed parameters.

> +
> +static int
> +dshow_list_device_options(AVFormatContext *avctx, ICreateDevEnum *devenum,
> +                          enum dshowDeviceType devtype)
> +{
> +    IBaseFilter *device_filter = NULL;
> +    int r;
> +
> +    if ((r = dshow_cycle_devices(avctx, devenum, devtype, &device_filter)) < 0)
> +        return r;
> +    if ((r = dshow_cycle_pins(avctx, devtype, device_filter, NULL)) < 0)
> +        return r;
>  
>      return 0;
>  }
> @@ -757,6 +801,14 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
>          ret = AVERROR_EXIT;
>          goto error;
>      }
> +    if (ctx->list_options) {
> +        if (ctx->device_name[VideoDevice])
> +            dshow_list_device_options(avctx, devenum, VideoDevice);
> +        if (ctx->device_name[AudioDevice])
> +            dshow_list_device_options(avctx, devenum, AudioDevice);
> +        ret = AVERROR_EXIT;
> +        goto error;
> +    }
>  
>      if (ctx->device_name[VideoDevice]) {
>          ret = dshow_open_device(avctx, devenum, VideoDevice);
> @@ -856,6 +908,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 },
>  };
>  
> -- 
> 1.7.4.1
> 

> From 49ec8b784660f1b802a3748ca8eaa7fec3c62cd8 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:15:41 -0300
> Subject: [PATCH 8/9] dshow: indent
> 
> ---
>  libavdevice/dshow.c |   24 +++++++++++++-----------
>  1 files changed, 13 insertions(+), 11 deletions(-)
[...]

> From 6d1bd1949f241dfdb61ab3ed94e9550018ba0f82 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Wed, 7 Sep 2011 21:21:01 -0300
> Subject: [PATCH 9/9] doc: add documentation for dshow indev
> 
> ---
>  doc/indevs.texi |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 77 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index c38031f..d0cb0e4 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -55,6 +55,83 @@ 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, it will
> +fail to open.
> +
> + 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
> +
> + at itemize
> +
> + at item Print the list of DirectShow supported devices and exit:
> + at example
> +$ ffmpeg -list_devices true -f dshow -i dummy
> + at end example
> +
> + at item Open video device @var{Camera}:
> + at example
> +$ ffmpeg -f dshow -i video="Camera"
> + at end example
> +
> + at item Open video device @var{Camera} and audio device @var{Microphone}:
> + at example
> +$ ffmpeg -f dshow -i video="Camera":audio="Microphone"
> + at end example
> +
> + at item 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
> +
> + at end itemize
> +
>  @section dv1394

Looks fine.
-- 
FFmpeg = Furious Funny Merciful Practical Evanescent Gorilla


More information about the ffmpeg-devel mailing list