[FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.
george at nsup.org
Mon Dec 19 10:40:41 EET 2016
L'octidi 28 frimaire, an CCXXV, wm4 a écrit :
> For buffersink, you could simply return a struct with the parameters.
> As a value type, it'd be a copy and wouldn't need accessors.
You mean a single structure returned by a single accessor with all the
stream's properties instead of individual accessors for individual
properties? Well, the naive methods of returning a structure to the
caller would make the size of the structure part of the API, but there
are ways around it.
I do not dislike the idea, if that is what people prefer.
(AVCodecParameter would have been an obvious candidate for that, but it
lacks a few necessary fields. And it has a lot of extra fields, which
goes against what you write below.)
> Since you moved the discussion to general API issues...
> Using public fields and such "internal" structs is functionally
> equivalent to having an opaque struct with trivial accessors. It's
> basically a style issue.
This is true, but "functionally equivalent" is a very myopic criterion.
It misses all the impact of the design on the code readability. Just
look at all the "->inputs" that this patch allows to eliminate (and
that James missed, I think).
> The difference between them is essentially syntax only. Both of them
> have multiple disadvantages:
> - too much to deal with at once (whether it's fields or
> setters/getters), no logical separation of functionality that is
> lesser needed or doesn't make sense in some contexts. (Consider all
> these AVCodecContext fields for tuning encoders - what do they do for
> decoding? What do fields, which are audio-only, do if video is
> encoded or decoded?)
It feels more a mater of documentation and auto-documentation than
anything else. "There is a crf field, does it apply to the x262 encder?"
and "There is a crf option, does it apply to the x262 encoder?" are
exactly the same question, the only difference being that options are
auto-documenting and answer the question automatically. But we could
have found a way for fields as well.
> - it's unclear when these fields can be set or not. (Why _can_ you set
> a field if the graph is already "configured"? What happens then? How
> is the user supposed to know when it's ok to set them?)
> - even if you argue that the previous point can be fixed by having the
> setters check the state and return an error value on misuse, it's
> complex for both API implementer and user
All considered, the complexity is in the task: video encoding is an
incredibly complex subject. API can only do so much to ease it.
> - many uses of this hide internal fields from the public API user,
> which is good. But at the same time, this moves away from the
> (probably worthy) goal of allowing outside implementation of
> codecs, (de)muxers, filters.
This calls to my recent answer to Michael about making AVFilterLink and
AVFilterPad opaque: allowing foreign modules requires stability for APIs
that are currently internal, and make development that much harder.
> So I would suggest that instead of changing the whole API to use
> accessors, we should make the API more "transactional", and force
> correct use by API design.
Unfortunately, if doing that was simple, we would already be doing it.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 833 bytes
Desc: Digital signature
More information about the ffmpeg-devel