On Tue, May 10, 2016 at 2:35 PM, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Tue, May 10, 2016 at 02:02:52PM +0200, Hendrik Leppkes wrote: >> On Tue, May 10, 2016 at 1:43 PM, Michael Niedermayer >> <mich...@niedermayer.cc> wrote: >> > On Tue, May 10, 2016 at 11:17:12AM +0100, Derek Buitenhuis wrote: >> >> On 5/10/2016 7:24 AM, Hendrik Leppkes wrote: >> >> > I don't like this, the struct is pretty cleanly defined and unlikely >> >> > to be extended much over time. >> >> > Most other structs have AVOptions so the CLI can interact with it, but >> >> > this struct is not meant to be modified by users, its just a direct >> >> > line of communication between demuxer->decoder or encoder->muxer. >> >> >> >> To me this mostly still feels like trying to make a demuxing library >> >> something it isn't. It demuxes and muxes. >> >> >> >> You can argue that it should be easier than decoding a frame to get this >> >> info (which I don't personally think is much trouble at all). However, >> >> jamming it into libavformat for reasons like "we've always done this" and >> >> "there's no better place" is not good design IMO. >> >> >> >> It sounds like what you really want is a higher level "easy" API. Don't >> >> jam it in to existing decoupled APIs because it is convenient, IMO. That's >> >> how horrible things like having ffmpeg.c functionality in libavformat came >> >> to exist, and a decade of misery followed. >> > >> > This patch adds an AVClass field to AVCodecParameters like we use in >> > other public structures. >> > i fail to see how this patch is related to your reply >> > >> > What the patch does, is it makes the API more consistent and easy to >> > use. Users call the AVOption functions to set fields in the main >> > public structures, if they do that for AVCodecParameters it would >> > crash. >> > Even an empty AVClass that doesnt list options would reduce that >> > misery by at least not crashing but giving a clear error message. >> > >> >> We should not design our API around people not caring to read our API, >> because thats not a fight we can ever win. > > APIs should be designed with the "Principle of least astonishment" > > its quite surprising that AVOption APIs work with most public API > structures but crash with some > (AVPacket is another and i am of the oppinon and stated that previously > IIRC that it too should have a AVClass as its first element, this is > just hard to add as it requires ABI bumping everything which is why > i didnt push much for that being done, it needs to be done at a > time we bump all stuff for more important reasons) >
The number of structs that don't have it probably greatly outnumber those that do. From the top of my head: AVFrame and AVSubtitle AVPacket AVStream AVCodecDescriptor AVCodecParameters, of course Any and all structs used for side-data (AVCPBProperties, AVPanScan, etc) In fact the only that do have it are those where it was originally used to access private options of the underlying components. AVFilterContext / AVFilterGraph AVCodecContext AVFormatContext So I'm not sure I can follow your argument of "everything else has it". - Hendrik _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
