[FFmpeg-devel] [PATCH] More DirectShow patches
Ramiro Polla
ramiro.polla at gmail.com
Sat Nov 5 18:00:38 CET 2011
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-dshow-support-choosing-between-devices-with-same-nam.patch
Type: application/octet-stream
Size: 3138 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-dshow-indent.patch
Type: application/octet-stream
Size: 840 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-dshow-release-filter-reference-obtained-from-enumera.patch
Type: application/octet-stream
Size: 1142 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-dshow-save-opened-device-reference-so-it-may-be-prop.patch
Type: application/octet-stream
Size: 992 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-dshow-don-t-print-min-max-values-for-fps-the-wrong-w.patch
Type: application/octet-stream
Size: 1144 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-dshow-support-BI_BITFIELDS-compression-type.patch
Type: application/octet-stream
Size: 1306 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0005.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-dshow-support-video-codec-and-pixel-format-selection.patch
Type: application/octet-stream
Size: 5427 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0006.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-dshow-fix-return-code-when-opening-device-fails.patch
Type: application/octet-stream
Size: 1506 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0007.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-dshow-set-frame-rate-to-prevent-fps-analysis-code-fr.patch
Type: application/octet-stream
Size: 1547 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0008.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-dshow-separate-user-and-device-configuration-errors.patch
Type: application/octet-stream
Size: 1860 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/92cf98cc/attachment-0009.obj>
More information about the ffmpeg-devel
mailing list