[FFmpeg-devel] [PATCH 1/4] lavf: add probe device API

[Nicolas George]

Le duodi 22 brumaire, an CCXXII, Lukasz Marek a écrit :
> Adds new API dedicated for output devices.
> 
> This API allows to
> - Test device with provided configuration
>   Audio devices params:
>     - device name
>     - codec
>     - sample format
>     - channel count
>     - channels layout
>     - sample rate
>   Video devices params:
>     - device name
>     - codec
>     - pixel format
>     - width (minimal width as input and real width as output)
>     - height (minimal height as input and real height as output)
> 
> - Get all supported configurations matching provided filter.
>   Filter may contain any subset of Audio/Video device params.

Just a few thought, especially linked to the compatibility of this API and
the cases I know best, i.e. mostly ALSA.

> 
> Behaviour of probe function can also be modified by flags.
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavformat/avformat.h |   97 +++++++++++++++++++++++++++++++++++++++++++++++-
>  libavformat/utils.c    |   72 +++++++++++++++++++++++++++++++++++
>  2 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 6bd54ce..68b9269 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -323,6 +323,28 @@ typedef struct AVFrac {
>      int64_t val, num, den;
>  } AVFrac;
>  
> +/**
> + *
> + */
> +typedef struct AVDeviceConfig {

> +    struct AVDeviceConfig *next;

Minor: the rest of the API usually returns an array rather than a linked
list.

> +    char* device_name;                  ///< device name, format depends on device
> +    char* device_description;           ///< human friendly name

Style, minor: * belongs with the variable name, not the type.

> +    enum AVCodecID codec;               ///< codec
> +    int format;                         ///< format (AVPixelFormat / AVSampleFormat)

> +    union {
> +        struct {  // audio parameters
> +            int sample_rate;            ///< [in]: expected sample rate,   [out]: device's default sample rate
> +            int channels;               ///< [in]: expected channel count, [out]: device's default channel count
> +            int64_t channel_layout;     ///< [in]: expected layout,        [out]: device's default layout
> +        };

> +        struct {  // video parameters
> +            int width;                  ///< [in]: required width,  [out]: maximum width
> +            int height;                 ///< [in]: required height, [out]: maximum height
> +        };

Some kind of frame rate information would probably be useful too.

> +    } stream_info;

IMHO, using an union is not worth the trouble.

> +} AVDeviceConfig;

I find this structure a bit strange: how does it express that the device
called "default" can support either stereo or 5.1? Or that /dev/video1 can
accept either RAWVIDEO/yuv420p or MJPEG?

I believe there are two dimensions in this kind of API: first, getting a
list of valid device names, to propose in GUIs; and, second, for any valid
device, getting the set of supported parameters.

Note that the first step may fail: for ALSA, the device name can be a
complex string containing parameters and chained plugins, building a
complete list is impossible. The same goes for devices that correspond to
entries in /dev: there is no way of knowing all automatic symlinks
configured on the system.

> +
>  /*************************************************/
>  /* input/output formats */
>  
> @@ -379,6 +401,14 @@ typedef struct AVProbeData {
>  
>  #define AVFMT_SEEK_TO_PTS   0x4000000 /**< Seeking is based on PTS */
>  
> +#define AV_PROBEDEV_DEFAULT_DEV                 0x0001  /**< Probe default device only */

> +#define AV_PROBEDEV_RAW_DATA                    0x0002  /**< Return only raw codecs: AV_CODEC_ID_PCM_*,
> +                                                                                     AV_CODEC_ID_RAWVIDEO */

What about hwaccel and similar?

> +#define AV_PROBEDEV_ALLOW_STREAM_PARAM_CHANGES  0x0004  /**< Allow modification of wanted stream
> +                                                             parameteres when provided value is not supported.
> +                                                             Video devices will update width and height for
> +                                                             screen resolution. */
> +

Allowing to request some kind of range would probably be more versatile:

    int width;     ///< preferred width
    int width_min; ///< minimum possible width
    int width_max; ///< maximum possible width

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131125/52f7ab51/attachment.asc>