On Sat, 6 Oct 2012 21:44:03 -0700, "Ronald S. Bultje" <[email protected]> 
wrote:
> Hi,
> 
> On Sat, Oct 6, 2012 at 9:19 PM, Anton Khirnov <[email protected]> wrote:
> >
> > On Sat, 6 Oct 2012 17:01:57 -0700, "Ronald S. Bultje" <[email protected]> 
> > wrote:
> >> Hi,
> >>
> >> On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov <[email protected]> wrote:
> >> > On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje" 
> >> > <[email protected]> wrote:
> >> >> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov <[email protected]> wrote:
> >> >> > ---
> >> >> >  avprobe.c         |    6 +++---
> >> >> >  cmdutils.c        |   16 +++++++++++-----
> >> >> >  tools/graph2dot.c |    4 ++--
> >> >> >  3 files changed, 16 insertions(+), 10 deletions(-)
> >> >> >
> >> >> > diff --git a/avprobe.c b/avprobe.c
> >> >> > index 16a5d29..3a3ae0f 100644
> >> >> > --- a/avprobe.c
> >> >> > +++ b/avprobe.c
> >> >> > @@ -584,6 +584,7 @@ static void show_stream(AVFormatContext *fmt_ctx, 
> >> >> > int stream_idx)
> >> >> >      const char *profile;
> >> >> >      char val_str[128];
> >> >> >      AVRational display_aspect_ratio;
> >> >> > +    const AVPixFmtDescriptor *desc;
> >> >> >
> >> >> >      probe_object_header("stream");
> >> >> >
> >> >> > @@ -629,9 +630,8 @@ static void show_stream(AVFormatContext *fmt_ctx, 
> >> >> > int stream_idx)
> >> >> >                            rational_string(val_str, sizeof(val_str), 
> >> >> > ":",
> >> >> >                            &display_aspect_ratio));
> >> >> >              }
> >> >> > -            probe_str("pix_fmt",
> >> >> > -                      dec_ctx->pix_fmt != AV_PIX_FMT_NONE ?
> >> >> > -                      av_pix_fmt_descriptors[dec_ctx->pix_fmt].name 
> >> >> > : "unknown");
> >> >> > +            desc = av_pix_fmt_desc_get(dec_ctx->pix_fmt);
> >> >>
> >> >> I think this is an awful idea, it should be OK to access the array 
> >> >> directly.
> >> >>
> >> >
> >> > I don't see any advantage to accessing the array directly.
> >>
> >> It often leads to smaller code. Instead of AVPixFmtDesc *d =
> >> getter(pix_fmt); if (!d) return -1; printf("%d\n", d->something);, you
> >> can just do printf("%d\n", array[pix_fmt].something);. Do we really
> >> need to babysit our users?
> >
> > Checking for d to be non-NULL is equivalent to checking that pix_fmt is
> > valid. So no, it does not necessarily makes the code longer.
> >>
> >> > It is the only case in all Libav where we export an array like this.
> >>
> >> We never supported MSVC. So we shouldn't?
> >>
> >
> > YES! =p
> >
> > Seriously though, there are only disadvantages to exporting the array
> > directly.
> >
> >> > It fixes the size of AVPixFmtDescriptor for no good reason.
> >>
> >> Is there a need to extend it?
> >>
> >
> > There might be. You don't want to find out that you need to extend it
> > two months after somebody else just bumped lavu.
> >
> >> > It prevents the caller from accessing all the descriptors when he's
> >> > using a newer library than the one he compiled against.
> >>
> >> if (compile_ver > runtime_ver) error(); on app init. Likely, you want
> >> to do that anyway.
> >>
> >> > And I hear there's some trouble with the evil microsoft thing and data
> >> > symbols.
> >>
> >> You have that anyway for any symbol shared between lavu/f/fi/c/r/*.
> >
> > It should be our long-term goal to remove those.
> 
> You can't. Stuff like ff_sqrt[], ff_log2[] etc. can't be removed. Never. 
> Sorry.
> 

The same way we can never have msvc support?

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to