Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On Sat, Oct 06, 2012 at 09:44:03PM -0700, Ronald S. Bultje wrote: > On Sat, Oct 6, 2012 at 9:19 PM, Anton Khirnov wrote: > > On Sat, 6 Oct 2012 17:01:57 -0700, "Ronald S. Bultje" > > wrote: > >> On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov wrote: > >> > 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 patches I sent are make-believe? Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On 10/07/2012 02:01 AM, Ronald S. Bultje wrote:
> 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?
It can be _quite_ bad (e.g. pix_fmt > PIX_FMT_NB)...
>> It is the only case in all Libav where we export an array like this.
>
> We never supported MSVC. So we shouldn't?
>
>> It fixes the size of AVPixFmtDescriptor for no good reason.
>
> Is there a need to extend it?
Yes =)
lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On 07/10/2012 1:16 AM, Diego Elio Pettenò wrote: > As I said this patch makes the ABI more stable, and distributions love > ABI stability, so please do not discount it simply because your use > cases don't care about it. We already have a V8 with its own ABI problems. I have to agree with Diego and Anton on this one. - Derek ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On 06/10/2012 22:11, Ronald S. Bultje wrote: >> > >> > The same way we can never have msvc support? > Right. > > Shall we move on to more practical patches? Sure, are we going to stop reviewing MSVC fixes? As I said this patch makes the ABI more stable, and distributions love ABI stability, so please do not discount it simply because your use cases don't care about it. We already have a V8 with its own ABI problems. -- Diego Elio Pettenò — Flameeyes flamee...@flameeyes.eu — http://blog.flameeyes.eu/ ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
Hi,
On Sat, Oct 6, 2012 at 9:47 PM, Anton Khirnov wrote:
>
> On Sat, 6 Oct 2012 21:44:03 -0700, "Ronald S. Bultje"
> wrote:
>> Hi,
>>
>> On Sat, Oct 6, 2012 at 9:19 PM, Anton Khirnov wrote:
>> >
>> > On Sat, 6 Oct 2012 17:01:57 -0700, "Ronald S. Bultje"
>> > wrote:
>> >> Hi,
>> >>
>> >> On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov wrote:
>> >> > On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje"
>> >> > wrote:
>> >> >> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov
>> >> >> 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?
Right.
Shall we move on to more practical patches?
Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On Sat, 6 Oct 2012 21:44:03 -0700, "Ronald S. Bultje"
wrote:
> Hi,
>
> On Sat, Oct 6, 2012 at 9:19 PM, Anton Khirnov wrote:
> >
> > On Sat, 6 Oct 2012 17:01:57 -0700, "Ronald S. Bultje"
> > wrote:
> >> Hi,
> >>
> >> On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov wrote:
> >> > On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje"
> >> > wrote:
> >> >> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov 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
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
Hi,
On Sat, Oct 6, 2012 at 9:19 PM, Anton Khirnov wrote:
>
> On Sat, 6 Oct 2012 17:01:57 -0700, "Ronald S. Bultje"
> wrote:
>> Hi,
>>
>> On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov wrote:
>> > On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje"
>> > wrote:
>> >> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov 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
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On Sat, 6 Oct 2012 17:01:57 -0700, "Ronald S. Bultje"
wrote:
> Hi,
>
> On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov wrote:
> > On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje"
> > wrote:
> >> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov 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.
--
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On 06/10/2012 17:01, Ronald S. Bultje wrote: >> > 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. > Actually Anton is referring to the opposite case (runtime > compile), which is pretty common for all distributions. If you were to do that, distributions would likely feel like kicking libav out for good. Speaking with my distro hat on. So I'm in favour of Anton's choice, ABI stability _is_ necessary and very welcome. -- Diego Elio Pettenò — Flameeyes flamee...@flameeyes.eu — http://blog.flameeyes.eu/ ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
Hi,
On Sat, Oct 6, 2012 at 1:28 PM, Anton Khirnov wrote:
> On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje"
> wrote:
>> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov 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?
> It is the only case in all Libav where we export an array like this.
We never supported MSVC. So we shouldn't?
> It fixes the size of AVPixFmtDescriptor for no good reason.
Is there a need to extend it?
> 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/*.
Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On 10/06/2012 09:35 PM, Ronald S. Bultje wrote:
> Hi,
>> @@ -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.
Why? You could provide a setting function if you think we should provide
a way to add entries on the fly (and remove them), beside that the
getter addresses the normal needs nicely IMHO.
lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On 10/06/2012 06:58 PM, Anton Khirnov wrote: > --- > avprobe.c |6 +++--- > cmdutils.c| 16 +++- > tools/graph2dot.c |4 ++-- > 3 files changed, 16 insertions(+), 10 deletions(-) > Seems fine. lu ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
On Sat, 6 Oct 2012 12:35:44 -0700, "Ronald S. Bultje"
wrote:
> Hi,
>
> On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov 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 is the only case in all Libav where we export an array like this.
It fixes the size of AVPixFmtDescriptor for no good reason.
It prevents the caller from accessing all the descriptors when he's
using a newer library than the one he compiled against.
And I hear there's some trouble with the evil microsoft thing and data
symbols.
--
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 4/9] tools: do not use av_pix_fmt_descriptors directly.
Hi,
On Sat, Oct 6, 2012 at 9:58 AM, Anton Khirnov 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.
Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
