On Sun, Jan 25, 2015 at 05:15:33PM +0100, wm4 wrote: > On Sun, 25 Jan 2015 13:39:10 +0100 > Michael Niedermayer <michae...@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 Döffinger wrote: > > > > On 25.01.2015, at 03:08, Michael Niedermayer <michae...@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); * requires more code to use once the first field of AVStream is made an AVClass compare: myctx->disposition = av_parse_disposition_flags(mystring); vs. av_opt_set(myctx, "disposition", mystring, 0); * Supports just a subset of the features: like "-forced" to remove the "forced" disposition type while leaving the other flags in place [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Opposition brings concord. Out of discord comes the fairest harmony. -- Heraclitus
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
