[FFmpeg-devel] [PATCH] Move av_parse_frame_size() and av_parse_frame_rate() from libavcodec to libavcore

[Michael Niedermayer]

On Sun, Jul 25, 2010 at 12:51:16PM +0200, Stefano Sabatini wrote:
> On date Sunday 2010-07-25 02:02:55 +0200, Michael Niedermayer encoded:
> > On Sat, Jul 24, 2010 at 01:36:28PM +0200, Stefano Sabatini wrote:
> > > On date Saturday 2010-07-24 04:18:22 +0200, Michael Niedermayer encoded:
> > > > On Fri, Jul 23, 2010 at 12:57:15PM +0200, Stefano Sabatini wrote:
[...]
> > > { "pal", (AVRational) {1, 25} },
> > >   
> > > > > +             return 0;
> > > > > +         }
> > > > > +
> > > > > +    /* Then, we try to parse it as fraction */
> > > > > +    cp = strchr(arg, '/');
> > > > > +    if (!cp)
> > > > > +        cp = strchr(arg, ':');
> > > > > +    if (cp) {
> > > > > +        char* cpp;
> > > > > +        frame_rate->num = strtol(arg, &cpp, 10);
> > > > > +        if (cpp != arg || cpp == cp)
> > > > > +            frame_rate->den = strtol(cp+1, &cpp, 10);
> > > > > +        else
> > > > > +           frame_rate->num = 0;
> > > > 
> > > > indention
> > > > {}
> > > > yes they are seperate from moving it of course ...
> > > > still i think you could try patcheck and fix the things
> > > > 
> > > > 
> > > > > +    }
> > > > [...]
> > > > > +
> > > > > +/**
> > > > > + * Parse str and put in width_ptr and height_ptr the detected values.
> > > > > + *
> > > > > + * @return 0 in case of a successful parsing, a negative value otherwise
> > > > 
> > > > this is unflexible for public abi
> > > > because it just says we will never return a value >0
> > > > doing so would break abi
> > > > if you say <0 is error >= 0 is no error then additional information could be
> > > > returned in the future without breaking abi/api
> > > 
> > > These comments are all unrelated to the proposed change, I'll fix them
> > > with other patches but after this patch is applied.
> > 
> > for being strictly correct, you have to fix the api/abi before making it
> > available from another lib
> 
> I don't see why this is necessary, the function was already public,
> doing it after or before makes no difference (apart from wasting my
> time, which I'd like to avoid).

I wasnt conciously aware that this function was public and used, if so you
will have to deal with the linker-versioning-workaround machinery.
reinhard should be able to explain how to do that without triggering
assert(0) in the linker


> 
> Also note that most FFmpeg functions return 0 in case of success, <0
> in case of failure, changing these functions behavior looks weird.

feel free to change all <0/=0 to <0/>=0

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100725/22cd0fa9/attachment.pgp>