On 5/10/16, Hendrik Leppkes <h.lepp...@gmail.com> wrote: > 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".
Everything should have it, it just takes few bytes ;-) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
