Re: [FFmpeg-devel] [PATCH] define AVPixelFormat aliases as enumerators instead of macros
On Fri, 15 Jan 2016 10:39:59 -0800
Richard Smith wrote:
> On Fri Jan 15 08:51:07 CET 2016 wm4 wrote;
> > On Thu, 14 Jan 2016 13:58:14 -0800 Richard Smith
> > wrote:
> > > libavutil/pixfmt.h defines a collection of endian-specific pixel formats
> > > as
> > > macros. These macro names can cause conflicts with external projects that
> > > use those identifiers for their own purposes. Here's a patch to define
> > > these aliases as enumerators instead of macros, please consider merging:
> > >
> > >
> > > https://github.com/zygoloid/FFmpeg/commit/c20a0e2e66e52c45b9193bc750165b7ecf7f3ca4
> > >
> > > (Note that AV_PIX_FMT_Y400A was already defined as an enumerator in the
> > > PixelFormat enumeration, so I deleted its (no-op) macro entirely.)
> >
> > API users might check for the existence of such pixfmts with #ifdef,
>
> That would be a very odd thing for them to do, as most pixfmts do not
> have #defines.
>
> > and I don't understand the reasoning behind your patch. Why would
> > external projects redefine these macros?
>
> The project in question has its own enumeration:
>
> namespace MyProject {
> enum PixelFormatToUse {
> // ... some other values ...
> AV_PIX_FMT_RGB32, // use ffmpeg's AV_PIX_FMT_RGB32
> // ...
> };
> }
Then maybe it shouldn't do that, or if it does, this source file
shouldn't include any ffmpeg related headers. This is just asking for
trouble big-time. Use a different prefix if you really want to do this.
FFmpeg uses preprocessor defines for a lot of things, and you just have
to expect that FFmpeg will add macros prefixed with AV_ as it pleases.
> The names are intentionally chosen to be in 1-1 correspondence with
> ffmpeg's names. But ffmpeg's macro sometimes renames this project's
> enumerator, depending on whether its header is included before that
> file.
This is not a question of what should be, it's question of
compatibility. We try not to break downstream projects unnecessarily.
I'd just suggest defining aliases with names that do not violate
ffmpeg's namespace.
If you want to do this change, you IMHO need to keep the defines under a
compatibility ifdef, that will remain active until the next major
library bump. (Major bumps are when we change the API/ABI in
incompatible ways. Look e.g. into libavutil/version.h for a bunch of
APIs that will be removed or changed on the next bump.)
I don't know if others shares my opinion whether compatibility is
needed here.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] define AVPixelFormat aliases as enumerators instead of macros
On Fri Jan 15 08:51:07 CET 2016 wm4 wrote;
> On Thu, 14 Jan 2016 13:58:14 -0800 Richard Smith
> wrote:
> > libavutil/pixfmt.h defines a collection of endian-specific pixel formats as
> > macros. These macro names can cause conflicts with external projects that
> > use those identifiers for their own purposes. Here's a patch to define
> > these aliases as enumerators instead of macros, please consider merging:
> >
> >
> > https://github.com/zygoloid/FFmpeg/commit/c20a0e2e66e52c45b9193bc750165b7ecf7f3ca4
> >
> > (Note that AV_PIX_FMT_Y400A was already defined as an enumerator in the
> > PixelFormat enumeration, so I deleted its (no-op) macro entirely.)
>
> API users might check for the existence of such pixfmts with #ifdef,
That would be a very odd thing for them to do, as most pixfmts do not
have #defines.
> and I don't understand the reasoning behind your patch. Why would
> external projects redefine these macros?
The project in question has its own enumeration:
namespace MyProject {
enum PixelFormatToUse {
// ... some other values ...
AV_PIX_FMT_RGB32, // use ffmpeg's AV_PIX_FMT_RGB32
// ...
};
}
The names are intentionally chosen to be in 1-1 correspondence with
ffmpeg's names. But ffmpeg's macro sometimes renames this project's
enumerator, depending on whether its header is included before that
file.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] define AVPixelFormat aliases as enumerators instead of macros
On 15 January 2016 at 18:51, wm4 wrote: > API users might check for the existence of such pixfmts with #ifdef, > and I don't understand the reasoning behind your patch. Why would > external projects redefine these macros? All other pixfmts are already enums, why the discrepency ? Having everything an enum seems much cleaner to me IMHO. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] define AVPixelFormat aliases as enumerators instead of macros
On Thu, Jan 14, 2016 at 10:58 PM, Richard Smith wrote: > libavutil/pixfmt.h defines a collection of endian-specific pixel formats as > macros. These macro names can cause conflicts with external projects that > use those identifiers for their own purposes. Here's a patch to define > these aliases as enumerators instead of macros, please consider merging: > > > https://github.com/zygoloid/FFmpeg/commit/c20a0e2e66e52c45b9193bc750165b7ecf7f3ca4 > > (Note that AV_PIX_FMT_Y400A was already defined as an enumerator in the > PixelFormat enumeration, so I deleted its (no-op) macro entirely.) In contrast to earlier times where this was not the case and rather generic names were defined, all those macros are properly namespaced AV_*, so any conflicts with other software can easily be avoided by not using our namespace for their own names. I don't see the point potentially breaking API compat for this. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] define AVPixelFormat aliases as enumerators instead of macros
On Thu, 14 Jan 2016 13:58:14 -0800 Richard Smith wrote: > libavutil/pixfmt.h defines a collection of endian-specific pixel formats as > macros. These macro names can cause conflicts with external projects that > use those identifiers for their own purposes. Here's a patch to define > these aliases as enumerators instead of macros, please consider merging: > > > https://github.com/zygoloid/FFmpeg/commit/c20a0e2e66e52c45b9193bc750165b7ecf7f3ca4 > > (Note that AV_PIX_FMT_Y400A was already defined as an enumerator in the > PixelFormat enumeration, so I deleted its (no-op) macro entirely.) API users might check for the existence of such pixfmts with #ifdef, and I don't understand the reasoning behind your patch. Why would external projects redefine these macros? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] define AVPixelFormat aliases as enumerators instead of macros
libavutil/pixfmt.h defines a collection of endian-specific pixel formats as macros. These macro names can cause conflicts with external projects that use those identifiers for their own purposes. Here's a patch to define these aliases as enumerators instead of macros, please consider merging: https://github.com/zygoloid/FFmpeg/commit/c20a0e2e66e52c45b9193bc750165b7ecf7f3ca4 (Note that AV_PIX_FMT_Y400A was already defined as an enumerator in the PixelFormat enumeration, so I deleted its (no-op) macro entirely.) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
