[FFmpeg-devel] Broken endian indication in pixfmt list

[Michael Niedermayer]

On Wed, Nov 05, 2014 at 03:37:04AM +0100, wm4 wrote:
> 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

these seem possible, yes
accessor(s) would be needed in the first case too though 


> 
> 
> 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.

yes, though there where changes previously already which changed it
see d2962e9f89cca6ff40f0c9d5ffc9c4397b8b1b26

also theres a potential hacky alternative, the last 2 chars of the
name could be checked for "le"/"be"


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141105/3a2ac18a/attachment.asc>