[FFmpeg-devel] [PATCH v5 13/13] avdevice/dshow: select format with extended color info

[Andreas Rheinhardt]

Diederick Niehorster:
> Some DirectShow devices (Logitech C920 webcam) expose each DirectShow
> format they support twice, once without and once with extended color
> information. During format selection, both match, this patch ensures
> that the format with extended color information is selected if it is
> available, else it falls back to a matching format without such
> information. This also necessitated a new code path taken for default
> formats of a device (when user didn't request any specific video size,
> etc), because the default format may be one without extended color
> information when a twin with extended color information is also
> available. Getting the extended color information when available is
> important as it allows setting the color space, range, primaries,
> transfer characteristics and chroma location of the stream provided by
> dshow, enabling users to get more correct color automatically out of
> their device.
> 
> Closes: #9271
> 
> Signed-off-by: Diederick Niehorster <dcnieho at gmail.com>
> ---
>  libavdevice/dshow.c | 469 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 338 insertions(+), 131 deletions(-)
> 

[...]

> @@ -1342,6 +1556,7 @@ dshow_add_device(AVFormatContext *avctx,
>      AM_MEDIA_TYPE type;
>      AVCodecParameters *par;
>      AVStream *st;
> +    struct dshow_format_info *fmt_info = NULL;
>      int ret = AVERROR(EIO);
>  
>      type.pbFormat = NULL;
> @@ -1356,12 +1571,14 @@ dshow_add_device(AVFormatContext *avctx,
>      ctx->capture_filter[devtype]->stream_index = st->index;
>  
>      ff_dshow_pin_ConnectionMediaType(ctx->capture_pin[devtype], &type);
> +    fmt_info = dshow_get_format_info(&type);
> +    if (!fmt_info)
> +        goto error;

It is dangerous to goto error without setting ret to an error; it might
work now due to the ret = AVERROR(EIO) initialization, but it is
nevertheless dangerous.

>  
>      par = st->codecpar;
>      if (devtype == VideoDevice) {
>          BITMAPINFOHEADER *bih = NULL;
>          AVRational time_base;
> -        DXVA2_ExtendedFormat *extended_format_info = NULL;
>  
>          if (IsEqualGUID(&type.formattype, &FORMAT_VideoInfo)) {
>              VIDEOINFOHEADER *v = (void *) type.pbFormat;
> @@ -1371,8 +1588,6 @@ dshow_add_device(AVFormatContext *avctx,
>              VIDEOINFOHEADER2 *v = (void *) type.pbFormat;
>              time_base = (AVRational) { v->AvgTimePerFrame, 10000000 };
>              bih = &v->bmiHeader;
> -            if (v->dwControlFlags & AMCONTROL_COLORINFO_PRESENT)
> -                extended_format_info = (DXVA2_ExtendedFormat *) &v->dwControlFlags;
>          }
>          if (!bih) {
>              av_log(avctx, AV_LOG_ERROR, "Could not get media type.\n");
> @@ -1383,33 +1598,21 @@ dshow_add_device(AVFormatContext *avctx,
>          st->r_frame_rate = av_inv_q(time_base);
>  
>          par->codec_type = AVMEDIA_TYPE_VIDEO;
> -        par->width      = bih->biWidth;
> -        par->height     = bih->biHeight;
> +        par->width      = fmt_info->width;
> +        par->height     = fmt_info->height;
>          par->codec_tag  = bih->biCompression;
> -        par->format     = dshow_pixfmt(bih->biCompression, bih->biBitCount);
> +        par->format     = fmt_info->pix_fmt;
>          if (bih->biCompression == MKTAG('H', 'D', 'Y', 'C')) {
>              av_log(avctx, AV_LOG_DEBUG, "attempt to use full range for HDYC...\n");
>              par->color_range = AVCOL_RANGE_MPEG; // just in case it needs this...
>          }
> -        if (extended_format_info) {
> -            par->color_range = dshow_color_range(extended_format_info);
> -            par->color_space = dshow_color_space(extended_format_info);
> -            par->color_primaries = dshow_color_primaries(extended_format_info);
> -            par->color_trc = dshow_color_trc(extended_format_info);
> -            par->chroma_location = dshow_chroma_loc(extended_format_info);
> -        }
> -        if (par->format == AV_PIX_FMT_NONE) {
> -            const AVCodecTag *const tags[] = { avformat_get_riff_video_tags(), NULL };
> -            par->codec_id = av_codec_get_id(tags, bih->biCompression);
> -            if (par->codec_id == AV_CODEC_ID_NONE) {
> -                av_log(avctx, AV_LOG_ERROR, "Unknown compression type. "
> -                                 "Please report type 0x%X.\n", (int) bih->biCompression);
> -                ret = AVERROR_PATCHWELCOME;
> -                goto error;
> -            }
> -            par->bits_per_coded_sample = bih->biBitCount;
> -        } else {
> -            par->codec_id = AV_CODEC_ID_RAWVIDEO;
> +        par->color_range = fmt_info->col_range;
> +        par->color_space = fmt_info->col_space;
> +        par->color_primaries = fmt_info->col_prim;
> +        par->color_trc = fmt_info->col_trc;
> +        par->chroma_location = fmt_info->chroma_loc;
> +        par->codec_id = fmt_info->codec_id;
> +        if (par->codec_id == AV_CODEC_ID_RAWVIDEO) {
>              if (bih->biCompression == BI_RGB || bih->biCompression == BI_BITFIELDS) {
>                  par->bits_per_coded_sample = bih->biBitCount;
>                  if (par->height < 0) {
> @@ -1422,23 +1625,26 @@ dshow_add_device(AVFormatContext *avctx,
>                      }
>                  }
>              }
> +        } else {
> +            if (par->codec_id == AV_CODEC_ID_NONE) {
> +                av_log(avctx, AV_LOG_ERROR, "Unknown compression type. "
> +                                 "Please report type 0x%X.\n", (int) bih->biCompression);
> +                ret = AVERROR_PATCHWELCOME;
> +                goto error;
> +            }
> +            par->bits_per_coded_sample = bih->biBitCount;
>          }
>      } else {
> -        WAVEFORMATEX *fx = NULL;
> -
> -        if (IsEqualGUID(&type.formattype, &FORMAT_WaveFormatEx)) {
> -            fx = (void *) type.pbFormat;
> -        }
> -        if (!fx) {
> +        if (!IsEqualGUID(&type.formattype, &FORMAT_WaveFormatEx)) {
>              av_log(avctx, AV_LOG_ERROR, "Could not get media type.\n");
>              goto error;
>          }
>  
>          par->codec_type  = AVMEDIA_TYPE_AUDIO;
> -        par->format      = sample_fmt_bits_per_sample(fx->wBitsPerSample);
> +        par->format      = sample_fmt_bits_per_sample(fmt_info->sample_size);
>          par->codec_id    = waveform_codec_id(par->format);
> -        par->sample_rate = fx->nSamplesPerSec;
> -        par->channels    = fx->nChannels;
> +        par->sample_rate = fmt_info->sample_rate;
> +        par->channels    = fmt_info->channels;
>      }
>  
>      avpriv_set_pts_info(st, 64, 1, 10000000);
> @@ -1446,6 +1652,7 @@ dshow_add_device(AVFormatContext *avctx,
>      ret = 0;
>  
>  error:
> +    av_freep(&fmt_info);
>      if (type.pbFormat)
>          CoTaskMemFree(type.pbFormat);
>      return ret;
>