[FFmpeg-devel] [PATCH] More DirectShow patches
Stefano Sabatini
stefasab at gmail.com
Tue Sep 4 18:25:46 CEST 2012
On date Thursday 2012-08-23 16:45:47 -0300, Ramiro Polla encoded:
> On Sat, Nov 5, 2011 at 8:49 PM, Stefano Sabatini <stefasab at gmail.com> wrote:
[...]
> > 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.
>
> I am no longer able to reproduce the issue this patch was trying to
> fix. It's either because I forgot how to reproduce it or some change
> regarding time_base some time ago fixed it.
>
> Ramiro
> From 68bf7d131ada9357108d31d7f18f7008319c8102 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Thu, 23 Aug 2012 16:35:55 -0300
> Subject: [PATCH 1/2] 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 93bca1d..d3b5d8f 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 "libavformat/internal.h"
> #include "avdevice.h"
> @@ -51,6 +52,9 @@ struct dshow_ctx {
>
> IMediaControl *control;
>
> + char *pixel_format_str;
> + enum PixelFormat pixel_format;
> + enum AVCodecID 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);
> + if (pix_fmt == PIX_FMT_NONE) {
> + enum AVCodecID codec_id = dshow_codecid(bih->biCompression);
> + AVCodec *c = (codec_id == AV_CODEC_ID_NONE) ?
> + NULL : avcodec_find_decoder(codec_id);
Nit: codec
Also simpler:
AVCodec *codec = avcodec_find_decoder(codec_id);
as codec will be NULL if codec_id == NONE.
> + if (codec_id == AV_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);
> + }
I wonder if these logs don't really belong to VERBOSE.
> + } else {
> + av_log(avctx, AV_LOG_INFO, " pix_fmt=%s", av_get_pix_fmt_name(pix_fmt));
> + }
> + 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);
> + av_log(avctx, AV_LOG_INFO, "\n");
Why a separate log line for this?
> continue;
> }
> + if (ctx->video_codec_id != AV_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->pixel_format != PIX_FMT_NONE for consistency?
> + ctx->video_codec_id != AV_CODEC_ID_RAWVIDEO))
> || (devtype == AudioDevice && (ctx->channels || ctx->sample_rate));
> int format_set = 0;
>
> @@ -801,6 +829,22 @@ static int dshow_read_header(AVFormatContext *avctx)
> goto error;
> }
>
> + ctx->video_codec_id = avctx->video_codec_id ? avctx->video_codec_id
> + : AV_CODEC_ID_RAWVIDEO;
> + if (ctx->pixel_format_str) {
> + if (ctx->video_codec_id != AV_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) {
> @@ -941,6 +985,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 },
Unrelated: you may use AV_OPT_VIDEO_SIZE
> + { "pixel_format", "set video pixel format", OFFSET(pixel_format_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
You may use AV_OPT_PIXEL_FMT
> { "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 f480590b658f36d3ca84c57111f4bd2af09c1fe3 Mon Sep 17 00:00:00 2001
> From: Ramiro Polla <ramiro.polla at gmail.com>
> Date: Thu, 23 Aug 2012 16:36:36 -0300
> Subject: [PATCH 2/2] 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 d3b5d8f..05e1bbe 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -895,20 +895,18 @@ static int dshow_read_header(AVFormatContext *avctx)
> }
>
> if (ctx->device_name[VideoDevice]) {
> - ret = dshow_open_device(avctx, devenum, VideoDevice);
> - if (ret < 0)
> - goto error;
> - ret = dshow_add_device(avctx, VideoDevice);
> - if (ret < 0)
> + if ((r = dshow_open_device(avctx, devenum, VideoDevice)) < 0 ||
> + (r = dshow_add_device(avctx, 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, AudioDevice);
> - if (ret < 0)
> + if ((r = dshow_open_device(avctx, devenum, AudioDevice)) < 0 ||
> + (r = dshow_add_device(avctx, AudioDevice) < 0)) {
> + ret = r;
> goto error;
> + }
> }
Uhm, this double r/ret business is confusing. Maybe it would be
simpler to separate normal terminating logic from error code logic.
Also after several minutes spent looking at the patch and at the code
I can't still figure out why this is necessary.
--
FFmpeg = Frightening & Furious Minimal Perfectionist Experimenting Gadget
More information about the ffmpeg-devel
mailing list