[FFmpeg-devel] [PATCH 2/4] lavd: add device capabilities API

[Lukasz M]

>
>
> > +    if (!(*caps))
> > +        return AVERROR(ENOMEM);
>
> > +    av_opt_set_defaults(*caps);
> > +    (*caps)->device_context = s;
> > +    if ((ret = av_opt_set_dict(s->priv_data, device_options)) < 0)
> > +        goto fail;
> > +    if (s->iformat) {
> > +        if ((ret = s->iformat->create_device_capabilities(s, *caps)) <
> 0)
> > +            goto fail;
> > +    } else {
> > +        if ((ret = s->oformat->create_device_capabilities(s, *caps)) <
> 0)
> > +            goto fail;
> > +    }
>
> Please correct me if I am wrong, but I believe that (*caps)->class is set
> by
> create_device_capabilities; therefore, the av_opt_set_defaults() need to
> come after that.
>

Yes, I moved it before sending a patch (I don't remember what I had in
mind), but it should be at the end.


>
> >  /**
> > + * Structure describes device settings.
> > + *
> > + * It is used by devices in conjuntion with av_device_capabilities
> AVOption table
> > + * to to implement capabilities probing API based on AVOption API.
> > + */
> > +typedef struct AVDeviceCapabilitiesQuery {
> > +    const AVClass *class;
> > +    char *device_name;
> > +    AVFormatContext *device_context;
> > +    enum AVCodecID codec;
>
> > +    int format;                          /**< AVSampleFormat or
> AVPixelFormat */
>
> I am not sure which is best: sharing the field or not sharing the field. It
> does not matter much.
>

It is shared in different structs.


> > +    /**
> > +     * Apply currently set device capabilities to device context.
> > +     * @see avdevice_capabilities_apply() for more details.
> > +     */
> > +    int (*apply_device_capabilities)(struct AVFormatContext *s, void
> *caps,
> > +                                     int strategy);
> > +    /**
> > +     * Free device capabilities submodule.
> > +     * @see avdevice_capabilities_free() for more details.
> > +     */
> > +    int (*free_device_capabilities)(struct AVFormatContext *s, void
> *caps);
> >  } AVOutputFormat;
> >  /**
> >   * @}
> > @@ -593,11 +609,31 @@ typedef struct AVInputFormat {
> >       * Active streams are all streams that have AVStream.discard <
> AVDISCARD_ALL.
> >       */
> >      int (*read_seek2)(struct AVFormatContext *s, int stream_index,
> int64_t min_ts, int64_t ts, int64_t max_ts, int flags);
>
> > +
>
> Spurious. Probably belongs in the previous patch.
>
> (What is the good way of moving a hunk from a patch to another?)
>

It don't know how to move hunk between patches. I usually reset one of them
and use git add -p and then amend or squash.


>
> >      /**
> >       * Returns device list with it properties.
> >       * @see avdevice_list_devices() for more details.
> >       */
> >      int (*get_device_list)(struct AVFormatContext *s, void
> **device_list);
>
> > +
> > +    /**
> > +     * Initialize device capabilities submodule.
> > +     * @see avdevice_capabilities_create() for more details.
> > +     */
> > +    int (*create_device_capabilities)(struct AVFormatContext *s, void
> *caps);
> > +
> > +    /**
> > +     * Apply currently set device capabilities to device context.
> > +     * @see avdevice_capabilities_apply() for more details.
> > +     */
> > +    int (*apply_device_capabilities)(struct AVFormatContext *s, void
> *caps,
> > +                                     int strategy);
> > +
> > +    /**
> > +     * Free device capabilities submodule.
> > +     * @see avdevice_capabilities_free() for more details.
> > +     */
> > +    int (*free_device_capabilities)(struct AVFormatContext *s, void
> *caps);
>
> Would it make sense to share these between in and out formats?
>

Hmmm, I don't understand how?


Updated patch attached
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-lavd-add-device-capabilities-API.patch
Type: text/x-patch
Size: 10875 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140211/a070c88f/attachment.bin>