[FFmpeg-devel] [PATCH] More DirectShow patches
Stefano Sabatini
stefasab at gmail.com
Sun Nov 6 00:49:52 CET 2011
On date Saturday 2011-11-05 15:00:38 -0200, Ramiro Polla encoded:
> Hi,
>
> On Tue, Oct 18, 2011 at 9:10 PM, Stefano Sabatini <stefasab at gmail.com> wrote:
> > On date Monday 2011-10-17 20:40:03 -0200, Ramiro Polla encoded:
> >> 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".
>
> Done.
>
> >> 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.
>
> Slightly reworded.
>
> >> 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.
>
> Slightly reworded.
>
> >> 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.
>
> Done.
>
> >> + 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");
>
> I prefer this way. The value in the struct is "compression type",
> which maps either to a pix_fmt or a codec_id in FFmpeg, so I test for
> pix_fmt, then codec_id, and if none are found it says "unknown
> compression type".
>
> >> + } 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)
>
> Done.
>
> > for consistency: pix_fmt=%s
>
> Added = to both pix_fmt and video_codec.
>
> >> + }
> >> + 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) { ...
>
> Done.
>
> >> + 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)
>
> Fixed, and added a new patch to fix more error codes.
>
> >> 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?
>
> Yes, but the error code of 0 would remain in 'ret' afterwards.
>
> >> 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.
>
> It's some code that reads a few frames before determining the fps.
> This makes it look like ffmpeg has stalled for a few seconds on
> startup. With this patch, it only reads one frame, so it stalls for
> much less time. I'm not sure this is the best solution, but it's what
> Michael suggested...
>
> All patches attached again.
>
> Ramiro
> From fef213c926d0fde5e624d2ced2003c5b4d9e0313 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 01/10] dshow: support choosing between devices with same name
>
> ---
> doc/indevs.texi | 14 ++++++++++++++
> libavdevice/dshow.c | 7 +++++++
> 2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index 3aa8f2f..90ae29c 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -104,6 +104,14 @@ 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,
> +defaults to 0).
> +
> + at item audio_device_number
> +Set audio device number for devices with same name (starts at 0,
> +defaults to 0).
> +
> @end table
>
> @subsection Examples
> @@ -123,6 +131,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 ab5f7e9..a5263df 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);
> @@ -938,6 +943,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 },
> };
>
> --
> 1.7.4.1
LGTM.
> From 27b3aaf6e9f16aa3138234caf147c3ccafa0d400 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 02/10] dshow: indent
>
> ---
> libavdevice/dshow.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index a5263df..4f641e7 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
OkkeyOfCourse.
> From 9d98663d6ecf66ebc58c74305bdfbfe7e8b948af 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 03/10] 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 4f641e7..0191d51 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
Should be fine.
> From 16287b44c4faf7e6ef08d7d72dc73620d8b9854a 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 04/10] dshow: save opened device reference so it may be properly closed
>
> ---
> libavdevice/dshow.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 0191d51..af2cf90 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;
>
> --
> 1.7.4.1
Yes, much better commit log.
> From 967242707d72581ab1dafb2dd501f0572e3dd400 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 05/10] 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 af2cf90..354e663 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.
> From 0dfa169600f1f11951d4f32f24bbd3f9fec510cb 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 06/10] dshow: support BI_BITFIELDS compression type
>
> ---
> libavdevice/dshow.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 354e663..bba1bba 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:
> @@ -711,7 +712,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
After reading this:
http://www.virtualdub.org/blog/pivot/entry.php?id=177
http://msdn.microsoft.com/en-us/library/dd183376%28v=vs.85%29.aspx
I wonder if the right format to select with biCompression =
BI_BITFIELDS should be detected against BITMAPINFO.bmiColors (this is
somehow similar to the way the pixel format is set in fbdev.c, you may
borrow some code from there).
> From 05cd8d1609ff06719524bc12bd8db91a9e0df127 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 07/10] dshow: support video codec and pixel format selection
>
> ---
> libavdevice/dshow.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index bba1bba..3bdf38c 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_str;
> + enum PixelFormat pixel_format;
> + 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);
> + }
> + } else {
> + av_log(avctx, AV_LOG_INFO, " (pix_fmt=%s)", av_get_pix_fmt_name(pix_fmt));
Nit: I still find the surrounding parentheses a bit weird...
> + }
> + 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_str &&
> + ctx->pixel_format != dshow_pixfmt(bih->biCompression, bih->biBitCount)) {
> + goto next;
> + }
> 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_str ||
> + ctx->video_codec_id != CODEC_ID_RAWVIDEO))
> || (devtype == AudioDevice && (ctx->channels || ctx->sample_rate));
> int format_set = 0;
>
> @@ -798,6 +826,22 @@ 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_str) {
> + 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");
> + ret = AVERROR(EINVAL);
> + goto error;
> + }
> + ctx->pixel_format = av_get_pix_fmt(ctx->pixel_format_str);
> + if (ctx->pixel_format == PIX_FMT_NONE) {
> + av_log(avctx, AV_LOG_ERROR, "No such pixel format '%s'.\n", ctx->pixel_format_str);
> + ret = AVERROR(EINVAL);
> + goto error;
> + }
> + }
> if (ctx->video_size) {
> r = av_parse_video_size(&ctx->requested_width, &ctx->requested_height, ctx->video_size);
> if (r < 0) {
> @@ -938,6 +982,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_str), 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
Looks nice otherwise (although missing docs, if you prefer you can do
it in another patch or I'll fill it in when committing).
> From da98929b19a22d85c2f5342a945c5aca62c1518f 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 08/10] 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 3bdf38c..c12847c 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -892,20 +892,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;
> + }
> }
OK, although I'd still prefer to always set explicitely the error code.
> ctx->mutex = CreateMutex(NULL, 0, NULL);
> --
> 1.7.4.1
>
> From 1e688cb216636689f3bf5372189eaa7bbc92c9df 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 09/10] 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 c12847c..09c6511 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -711,12 +711,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) {
> @@ -724,6 +727,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;
> --
> 1.7.4.1
Looks fine. I'd add this to the commit message to clarify the rationale:
|fps analysis makes it look like ffmpeg has stalled for a few seconds
|on startup when opening the dshow device. With this patch, it only
|reads one frame, so it stalls for much less time.
> From bfda37ef5babebb04166faebdc6acf00bee3f5ea Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Sat, 5 Nov 2011 14:50:18 -0200
> Subject: [PATCH 10/10] dshow: separate user and device configuration errors
>
> ---
> libavdevice/dshow.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 09c6511..39f312e 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -824,7 +824,8 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
> IGraphBuilder *graph = NULL;
> ICreateDevEnum *devenum = NULL;
> IMediaControl *control = NULL;
> - int ret = AVERROR(EIO);
> + /* Initial errors are related to user configuration. */
> + int ret = AVERROR(EINVAL);
> int r;
>
> if (!ctx->list_devices && !parse_device_name(avctx)) {
> @@ -838,13 +839,11 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
> 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");
> - ret = AVERROR(EINVAL);
> goto error;
> }
> ctx->pixel_format = av_get_pix_fmt(ctx->pixel_format_str);
> if (ctx->pixel_format == PIX_FMT_NONE) {
> av_log(avctx, AV_LOG_ERROR, "No such pixel format '%s'.\n", ctx->pixel_format_str);
> - ret = AVERROR(EINVAL);
> goto error;
> }
> }
> @@ -863,6 +862,9 @@ static int dshow_read_header(AVFormatContext *avctx, AVFormatParameters *ap)
> }
> }
>
> + /* From now on, errors are related to device configuration. */
> + ret = AVERROR(EIO);
> +
> CoInitialize(0);
>
> r = CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER,
Fine, or alternatively always set explicitely the error code (more
robust to changes, more predictable).
I'll apply the patches which LG(TM) in a day or so if I read no more
comments (and you don't beat me at it assuming you want to commit them
yourself), thanks.
--
FFmpeg = Friendly & Friendly Mastering Patchable Energized Glue
More information about the ffmpeg-devel
mailing list