[FFmpeg-devel] Broken endian indication in pixfmt list

[wm4]

On Wed, 5 Nov 2014 03:19:04 +0100
Michael Niedermayer <michaelni at gmx.at> wrote:

> On Wed, Nov 05, 2014 at 02:59:45AM +0100, wm4 wrote:
> > On Wed, 5 Nov 2014 02:44:12 +0100
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > > On Wed, Nov 05, 2014 at 02:13:29AM +0100, wm4 wrote:
> > > > The header pixfmt.h contains the following comment in the pixel format
> > > > list doxygen:
> > > > 
> > > >  * @note
> > > >  * Make sure that all newly added big-endian formats have (pix_fmt & 1) == 1
> > > >  * and that all newly added little-endian formats have (pix_fmt & 1) == 0.
> > > >  * This allows simpler detection of big vs little-endian.
> > > > 
> > > 
> > > > This is currently broken for the following formats: nv20le/be gbrap16le/be
> > > 
> > > yes
> > > 
> > > 
> > > > 
> > > > Obviously, this can't be fixed until the next ABI bump. But more
> > > 
> > > can you post a patch which does make sure it gets fixed then?
> > > and or that documentation corrected
> > 
> > As I said, I could write a patch that adds a proper flag, which the
> > user can use to determine endian properties. I'd also adjust the
> > documentation and inform the user that the old way doesn't work anymore.
> 
> iam fine with that but you might run into some obstacles
> the flags are uint8_t and all used
> adding a new field might require some care to ensure ABI compatibility
> and might need accessor functions

I didn't realize that. It sort of ruins the idea.

I see two solutions without breaking ABI:
- add a "flags2" member, which contains the flags; duplicate it, and
  deprecate the old "flags" member
- store the high bits of the "flags" member in a new "flags2" member,
  and add an accessor which returns the full flags


There's also the deprecated av_pix_fmt_descriptors[] variable - if we
care about this, extending AVPixFmtDescriptor is not possible without
breaking ABI. And currently this symbol is actually exported.

> 
> > 
> > I'm not so much interested in creating big ifdef messes.
> 
> neither am i
> 
> [...]
>