[FFmpeg-devel] [PATCH] More DirectShow patches
Stefano Sabatini
stefasab at gmail.com
Wed Oct 19 01:10:39 CEST 2011
On date Monday 2011-10-17 20:40:03 -0200, Ramiro Polla encoded:
> $subj
> From 1184e9336e50fe8adc74d3151244c985f84e886f Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Fri, 30 Sep 2011 17:49:09 -0300
> Subject: [PATCH 1/9] dshow: support choosing between devices with same name
>
> ---
> doc/indevs.texi | 12 ++++++++++++
> libavdevice/dshow.c | 7 +++++++
> 2 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index 79f786f..1f3739b 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -104,6 +104,12 @@ If set to @option{true}, print a list of devices and exit.
> If set to @option{true}, print a list of selected device's options
> and exit.
>
> + at item video_device_number
> +Set video device number for devices with same name (starts at 0).
> +
> + at item audio_device_number
> +Set audio device number for devices with same name (starts at 0).
> +
Maybe add "default to 0".
> @end table
>
> @subsection Examples
> @@ -123,6 +129,12 @@ $ ffmpeg -f dshow -i video="Camera"
> @end example
>
> @item
> +Open second video device with name @var{Camera}:
> + at example
> +$ ffmpeg -f dshow -video_device_number 1 -i video="Camera"
> + at end example
> +
> + at item
> Open video device @var{Camera} and audio device @var{Microphone}:
> @example
> $ ffmpeg -f dshow -i video="Camera":audio="Microphone"
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 6a77f4b..a258430 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -31,6 +31,8 @@ struct dshow_ctx {
> IGraphBuilder *graph;
>
> char *device_name[2];
> + int video_device_number;
> + int audio_device_number;
>
> int list_options;
> int list_devices;
> @@ -249,6 +251,8 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum,
> IEnumMoniker *classenum = NULL;
> IMoniker *m = NULL;
> const char *device_name = ctx->device_name[devtype];
> + int skip = (devtype == VideoDevice) ? ctx->video_device_number
> + : ctx->audio_device_number;
> int r;
>
> const GUID *device_guid[2] = { &CLSID_VideoInputDeviceCategory,
> @@ -283,6 +287,7 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum,
> if (strcmp(device_name, buf))
> goto fail1;
>
> + if (!skip--)
> IMoniker_BindToObject(m, 0, 0, &IID_IBaseFilter, (void *) &device_filter);
> } else {
> av_log(avctx, AV_LOG_INFO, " \"%s\"\n", buf);
> @@ -937,6 +942,8 @@ static const AVOption options[] = {
> { "list_options", "list available options for specified device", OFFSET(list_options), AV_OPT_TYPE_INT, {.dbl=0}, 0, 1, DEC, "list_options" },
> { "true", "", 0, AV_OPT_TYPE_CONST, {.dbl=1}, 0, 0, DEC, "list_options" },
> { "false", "", 0, AV_OPT_TYPE_CONST, {.dbl=0}, 0, 0, DEC, "list_options" },
> + { "video_device_number", "set video device number for devices with same name (starts at 0)", OFFSET(video_device_number), AV_OPT_TYPE_INT, {.dbl = 0}, 0, INT_MAX, DEC },
> + { "audio_device_number", "set audio device number for devices with same name (starts at 0)", OFFSET(audio_device_number), AV_OPT_TYPE_INT, {.dbl = 0}, 0, INT_MAX, DEC },
> { NULL },
> };
Seems fine.
> From 325724bc80b8ee467d90a3ad4480742db163e86f Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Fri, 30 Sep 2011 17:49:22 -0300
> Subject: [PATCH 2/9] dshow: indent
>
> ---
> libavdevice/dshow.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index a258430..d7cad2d 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -288,7 +288,7 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum,
> goto fail1;
>
> if (!skip--)
> - IMoniker_BindToObject(m, 0, 0, &IID_IBaseFilter, (void *) &device_filter);
> + IMoniker_BindToObject(m, 0, 0, &IID_IBaseFilter, (void *) &device_filter);
> } else {
> av_log(avctx, AV_LOG_INFO, " \"%s\"\n", buf);
> }
> --
> 1.7.4.1
Fine of course.
> From 38eaeb3096f30468117df1c9c969f75f46bb98ec Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Fri, 30 Sep 2011 17:49:43 -0300
> Subject: [PATCH 3/9] dshow: release filter reference obtained from enumeration
>
> ---
> libavdevice/dshow.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index d7cad2d..acfb7e9 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -121,10 +121,12 @@ dshow_read_close(AVFormatContext *s)
> if (r == S_OK) {
> IBaseFilter *f;
> IEnumFilters_Reset(fenum);
> - while (IEnumFilters_Next(fenum, 1, &f, NULL) == S_OK)
> + while (IEnumFilters_Next(fenum, 1, &f, NULL) == S_OK) {
> if (IGraphBuilder_RemoveFilter(ctx->graph, f) == S_OK)
> IEnumFilters_Reset(fenum); /* When a filter is removed,
> * the list must be reset. */
> + IBaseFilter_Release(f);
> + }
> IEnumFilters_Release(fenum);
> }
> IGraphBuilder_Release(ctx->graph);
> --
> 1.7.4.1
Seems OK.
> From dcdf0ae08f612d91a6338d02965fb9de936eaece Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Fri, 30 Sep 2011 17:50:00 -0300
> Subject: [PATCH 4/9] dshow: properly close device on option listing
>
> ---
> libavdevice/dshow.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index acfb7e9..2e60efa 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -561,11 +561,13 @@ static int
> dshow_list_device_options(AVFormatContext *avctx, ICreateDevEnum *devenum,
> enum dshowDeviceType devtype)
> {
> + struct dshow_ctx *ctx = avctx->priv_data;
> IBaseFilter *device_filter = NULL;
> int r;
>
> if ((r = dshow_cycle_devices(avctx, devenum, devtype, &device_filter)) < 0)
> return r;
> + ctx->device_filter[devtype] = device_filter;
> if ((r = dshow_cycle_pins(avctx, devtype, device_filter, NULL)) < 0)
> return r;
Seems OK, but the log is a bit backward, I suppose the logic is
something along the lines of:
|In function dshow_list_device_options(), set created device_filter in
|ctx->device_filter[devtype], so its reference can be released later.
> From 70482b24f7d22103a0bdf97e8dbaaef6e4e5ea76 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Fri, 30 Sep 2011 18:10:30 -0300
> Subject: [PATCH 5/9] dshow: don't print min/max values for fps the wrong way around
>
> ---
> libavdevice/dshow.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 2e60efa..8681fa9 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -371,9 +371,9 @@ dshow_cycle_formats(AVFormatContext *avctx, enum dshowDeviceType devtype,
> if (!pformat_set) {
> av_log(avctx, AV_LOG_INFO, " min s=%ldx%ld fps=%g max s=%ldx%ld fps=%g\n",
> vcaps->MinOutputSize.cx, vcaps->MinOutputSize.cy,
> - 1e7 / vcaps->MinFrameInterval,
> + 1e7 / vcaps->MaxFrameInterval,
> vcaps->MaxOutputSize.cx, vcaps->MaxOutputSize.cy,
> - 1e7 / vcaps->MaxFrameInterval);
> + 1e7 / vcaps->MinFrameInterval);
> continue;
> }
> if (ctx->framerate) {
> --
> 1.7.4.1
looks fine, alternatively you may want to rename the tags (fps ->
frame_interval), but that's bikeshed.
> From 56716cdbc1b35dbd6b0bae2ecbcb3ef50ebef0a0 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Sat, 8 Oct 2011 15:00:00 -0300
> Subject: [PATCH 6/9] dshow: support BI_BITFIELDS
>
> ---
> libavdevice/dshow.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 8681fa9..666e186 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -72,6 +72,7 @@ static enum PixelFormat dshow_pixfmt(DWORD biCompression, WORD biBitCount)
> return PIX_FMT_YUYV422;
> case MKTAG('I', '4', '2', '0'):
> return PIX_FMT_YUV420P;
> + case BI_BITFIELDS:
> case BI_RGB:
> switch(biBitCount) { /* 1-8 are untested */
> case 1:
> @@ -710,7 +711,7 @@ dshow_add_device(AVFormatContext *avctx, AVFormatParameters *ap,
> codec->bits_per_coded_sample = bih->biBitCount;
> } else {
> codec->codec_id = CODEC_ID_RAWVIDEO;
> - if (bih->biCompression == BI_RGB) {
> + if (bih->biCompression == BI_RGB || bih->biCompression == BI_BITFIELDS) {
> codec->bits_per_coded_sample = bih->biBitCount;
> codec->extradata = av_malloc(9 + FF_INPUT_BUFFER_PADDING_SIZE);
> if (codec->extradata) {
> --
> 1.7.4.1
OK, just please provide some more information related to the meaning of
BI_BITFIELDS.
> From a565deeba59d0f180872c15f8b3a8125bd7985a1 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Sat, 8 Oct 2011 15:02:09 -0300
> Subject: [PATCH 7/9] dshow: support video codec and pixel format selection
>
> ---
> libavdevice/dshow.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 666e186..d227539 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -20,6 +20,7 @@
> */
>
> #include "libavutil/parseutils.h"
> +#include "libavutil/pixdesc.h"
> #include "libavutil/opt.h"
>
> #include "avdevice.h"
> @@ -51,6 +52,9 @@ struct dshow_ctx {
>
> IMediaControl *control;
>
> + char *pixel_format;
> + enum PixelFormat pix_fmt;
nit: pixel_format and pixel_format_str to make clear the relation
between the two variables.
> + enum CodecID video_codec_id;
> char *video_size;
> char *framerate;
>
> @@ -370,13 +374,35 @@ dshow_cycle_formats(AVFormatContext *avctx, enum dshowDeviceType devtype,
> goto next;
> }
> if (!pformat_set) {
> - av_log(avctx, AV_LOG_INFO, " min s=%ldx%ld fps=%g max s=%ldx%ld fps=%g\n",
> + enum PixelFormat pix_fmt = dshow_pixfmt(bih->biCompression, bih->biBitCount);
> + av_log(avctx, AV_LOG_INFO, " min s=%ldx%ld fps=%g max s=%ldx%ld fps=%g",
> vcaps->MinOutputSize.cx, vcaps->MinOutputSize.cy,
> 1e7 / vcaps->MaxFrameInterval,
> vcaps->MaxOutputSize.cx, vcaps->MaxOutputSize.cy,
> 1e7 / vcaps->MinFrameInterval);
> + if (pix_fmt == PIX_FMT_NONE) {
> + enum CodecID codec_id = dshow_codecid(bih->biCompression);
> + AVCodec *c = (codec_id == CODEC_ID_NONE) ?
> + NULL : avcodec_find_decoder(codec_id);
> + if (codec_id == CODEC_ID_NONE || !c) {
> + av_log(avctx, AV_LOG_INFO, " (unknown compression type)");
> + } else {
> + av_log(avctx, AV_LOG_INFO, " (video_codec %s)", c->name);
> + }
nit:
simpler:
av_log(avctx, AV_LOG_INFO, " video_codec=%s", c ? c->name, "unknown");
> + } else {
> + av_log(avctx, AV_LOG_INFO, " (pix_fmt %s)", av_pix_fmt_descriptors[pix_fmt].name);
nit:
slightly shorter: av_get_pix_fmt_name(pix_fmt)
for consistency: pix_fmt=%s
> + }
> + av_log(avctx, AV_LOG_INFO, "\n");
> continue;
> }
> + if (ctx->video_codec_id != CODEC_ID_RAWVIDEO) {
> + if (ctx->video_codec_id != dshow_codecid(bih->biCompression))
> + goto next;
> + }
> + if (ctx->pixel_format) {
> + if (ctx->pix_fmt != dshow_pixfmt(bih->biCompression, bih->biBitCount))
> + goto next;
> + }
nit+: here you could merge the two if conditions: if (A) { if (B) ... -> if (A && B) { ...
> if (ctx->framerate) {
> int64_t framerate = ((int64_t) ctx->requested_framerate.den*10000000)
> / ctx->requested_framerate.num;
> @@ -465,7 +491,9 @@ 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))
> + int set_format = (devtype == VideoDevice && (ctx->video_size || ctx->framerate ||
> + ctx->pixel_format ||
> + ctx->video_codec_id != CODEC_ID_RAWVIDEO))
> || (devtype == AudioDevice && (ctx->channels || ctx->sample_rate));
> int format_set = 0;
>
> @@ -797,6 +825,20 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
> goto error;
> }
>
> + ctx->video_codec_id = avctx->video_codec_id ? avctx->video_codec_id
> + : CODEC_ID_RAWVIDEO;
> + if (ctx->pixel_format) {
> + if (ctx->video_codec_id != CODEC_ID_RAWVIDEO) {
> + av_log(avctx, AV_LOG_ERROR, "Pixel format may only be set when "
> + "video codec is not set or set to rawvideo.\n");
> + goto error;
> + }
> + ctx->pix_fmt = av_get_pix_fmt(ctx->pixel_format);
> + if (ctx->pix_fmt == PIX_FMT_NONE) {
> + av_log(avctx, AV_LOG_ERROR, "No such pixel format '%s'.\n", ctx->pixel_format);
> + goto error;
> + }
> + }
missing ret = AVERROR(EINVAL) (this is supposed to be an user configuration error)
> if (ctx->video_size) {
> r = av_parse_video_size(&ctx->requested_width, &ctx->requested_height, ctx->video_size);
> if (r < 0) {
> @@ -937,6 +979,7 @@ static int dshow_read_packet(AVFormatContext *s, AVPacket *pkt)
> #define DEC AV_OPT_FLAG_DECODING_PARAM
> static const AVOption options[] = {
> { "video_size", "set video size given a string such as 640x480 or hd720.", OFFSET(video_size), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> + { "pixel_format", "set video pixel format", OFFSET(pixel_format), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> { "framerate", "set video frame rate", OFFSET(framerate), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> { "sample_rate", "set audio sample rate", OFFSET(sample_rate), AV_OPT_TYPE_INT, {.dbl = 0}, 0, INT_MAX, DEC },
> { "sample_size", "set audio sample size", OFFSET(sample_size), AV_OPT_TYPE_INT, {.dbl = 0}, 0, 16, DEC },
> --
> 1.7.4.1
>
> From b1bc428df2ef85b13de09d7267200da13841b848 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Mon, 17 Oct 2011 12:17:58 -0200
> Subject: [PATCH 8/9] dshow: fix return code when opening device fails
>
> ---
> libavdevice/dshow.c | 18 ++++++++----------
> 1 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index d227539..48c2ffd 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -889,20 +889,18 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
> }
>
> if (ctx->device_name[VideoDevice]) {
> - ret = dshow_open_device(avctx, devenum, VideoDevice);
> - if (ret < 0)
> - goto error;
> - ret = dshow_add_device(avctx, ap, VideoDevice);
> - if (ret < 0)
> + if ((r = dshow_open_device(avctx, devenum, VideoDevice)) < 0 ||
> + (r = dshow_add_device(avctx, ap, VideoDevice) < 0)) {
> + ret = r;
> goto error;
> + }
> }
>
> if (ctx->device_name[AudioDevice]) {
> - ret = dshow_open_device(avctx, devenum, AudioDevice);
> - if (ret < 0)
> - goto error;
> - ret = dshow_add_device(avctx, ap, AudioDevice);
> - if (ret < 0)
> + if ((r = dshow_open_device(avctx, devenum, AudioDevice)) < 0 ||
> + (r = dshow_add_device(avctx, ap, AudioDevice) < 0)) {
> + ret = r;
> goto error;
> + }
> }
Isn't dshow_open/add_device() already returning an AVERROR() error
code?
> ctx->mutex = CreateMutex(NULL, 0, NULL);
> --
> 1.7.4.1
>
> From c28b467bcefd771d03663a4acc01f0c41a86e8be Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Mon, 17 Oct 2011 20:25:27 -0200
> Subject: [PATCH 9/9] dshow: set frame rate to prevent fps analysis code from running
>
> ---
> libavdevice/dshow.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 48c2ffd..284e3bf 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -710,12 +710,15 @@ dshow_add_device(AVFormatContext *avctx, AVFormatParameters *ap,
> codec = st->codec;
> if (devtype == VideoDevice) {
> BITMAPINFOHEADER *bih = NULL;
> + AVRational frame_rate;
>
> if (IsEqualGUID(&type.formattype, &FORMAT_VideoInfo)) {
> VIDEOINFOHEADER *v = (void *) type.pbFormat;
> + frame_rate = av_d2q(10000000. / v->AvgTimePerFrame, INT_MAX);
> bih = &v->bmiHeader;
> } else if (IsEqualGUID(&type.formattype, &FORMAT_VideoInfo2)) {
> VIDEOINFOHEADER2 *v = (void *) type.pbFormat;
> + frame_rate = av_d2q(10000000. / v->AvgTimePerFrame, INT_MAX);
> bih = &v->bmiHeader;
> }
> if (!bih) {
> @@ -723,6 +726,9 @@ dshow_add_device(AVFormatContext *avctx, AVFormatParameters *ap,
> goto error;
> }
>
> + st->avg_frame_rate = frame_rate;
> + st->r_frame_rate = frame_rate;
> +
> codec->time_base = ap->time_base;
> codec->codec_type = AVMEDIA_TYPE_VIDEO;
> codec->width = bih->biWidth;
Looks sane (but I'm not really sure about what "fps analysis code")
refers to.
More information about the ffmpeg-devel
mailing list