Hi,
On Sat, Oct 6, 2012 at 9:19 PM, Anton Khirnov <an...@khirnov.net> wrote:
>
> On Sat, 6 Oct 2012 17:01:57 -0700, "Ronald S. Bultje" <rsbul...@gmail.com>
> wrote:
>> Hi,
>>
>> On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov <an...@khirnov.net> wrote:
>> > On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje" <rsbul...@gmail.com>
>> > wrote:
>> >> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov <an...@khirnov.net> 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.
Ronald
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
