[FFmpeg-devel] [PATCH] Move stream_options to avformat

[Paul B Mahol]

On 1/25/15, wm4 <nfxjfg at googlemail.com> wrote:
> On Sun, 25 Jan 2015 13:39:10 +0100
> Michael Niedermayer <michaelni at gmx.at> wrote:
>
>> On Sun, Jan 25, 2015 at 01:18:31PM +0100, Michael Niedermayer wrote:
>> > On Sun, Jan 25, 2015 at 12:15:40PM +0100, Reimar Doeffinger wrote:
>> > > On 25.01.2015, at 03:08, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > > > On Sun, Jan 25, 2015 at 02:31:33AM +0100, wm4 wrote:
>> > > >>
>> > > >>>>
>> > > >>>> As an experienced API user, I don't have the slightest clue what
>> > > >>>> I'd do
>> > > >>>> with this API, or where to find information about it.
>> > > >>>
>> > > >>> the primary goal is to remove duplicated disposition type tables,
>> > > >>> which needs one of the tables to be public first
>> > > >>>
>> > > >>> [...]
>> > > >>
>> > > >> And this is the most awkward way you could find to do this?
>> > > >
>> > > > No, i could certainly find a more akward way, if people prefer
>> > > >
>> > > > this is just the way that would be a big step towards consistent
>> > > > and simple access to the structs
>> > > > All public structs use AVClass and AVOptions to allow applications
>> > > > to extract/enumerate fields except a few like AVStream
>> > > > this patch would add these AVClass & AVOption for AVStream, its
>> > > > indeed not populated for all fields and AVStream doesnt have a
>> > > > AVClass as its first field due to ABI. But its a step toward it
>> > > >
>> > > > Would people prefer that each field in AVStream has a custom and
>> > > > different way to access it, as long as it looks simpler when looked
>> > > > at in isolation ?
>> > >
>> > > Sorry if it's useless of me to only state some obvious questions, but:
>> > > I think it's clear we all want a simple, obvious and consistent API :)
>> > > If it's a bit messy, might there be a point in holding off a bit so we
>> > > aren't stuck with something complicated?
>> > > Could possibly another approach after a major bump be nicer?
>> > > Or maybe better documentation/examples?
>> >
>> > > I think this started with a valid complaint/concern but unfortunately
>> > > no better alternative, could we stick to considering that instead of
>> > > going over to agressive rhethoric?
>> >
>> > absolutley
>> > i would strongly prefer if others could take this over, my interrest
>> > was just in the technical side and i wanted to move AVStream to
>> > the same system we use for all other structs. As well as fixing the
>> > quite valid issue nicolas had raised with the duplicated tables.
>> > I am quite surprised that others dont see this as a clear and
>> > uncontroversal step, there really are just
>> > 1. If we want AVStream to be consistent with other structs, that means
>> >    AVOption & AVClass. And this patch is a step toward it, one could
>> >    make a bigger or smaller step but its then either more or less
>> >    code not different code.
>> > 2. There could be a different system be used for this field or for
>> >    AVStream, this would be inconsistent
>> > 3. We can implement both a system based on AVOptions/AVClass and a
>> >    system without them, why would this field that noone cared about
>> >    until now need this, iam not sure though
>> > 4. We can leave the triplicated tables as is and hope not to forget
>> >    updating them in sync
>> >
>> > To me the best choice is clear, move toward the same system we use
>> > elsewhere. Change that system everywhere if it could be improved
>> > I see nothing controversal on this patch but others do apparently.
>> > As i dont see what issue people have with this, i certainly cannot
>> > help fixing the patch. But iam happy to review & approve the solution
>> > that people do prefer
>>
>> About the documentation & example side, i dont think this should yet
>> be used from outside, its only a partial implementation of AVOption
>> for AVStream, a full implementation needs a ABI bump due to the
>> first field needing to be a AVClass
>>
>> [...]
>>
>
> How is it even consistent with "other structs"? Doesn't it just resolve
> flags? Resolving flags with a complicated AVOption contraption (which
> every user has to understand and duplicate) doesn't seem like a good
> choice to me at all. I hear about API users fighting with the basics of
> the FFmpeg API because it's so weird and complicated; seeing patches
> like this just feel like a bad joke in contrast.
>
> What's wrong with:
>
>     int av_parse_disposition_flags(const char *s);
>
> ?

How than one can know which flags are available?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>