[FFmpeg-devel] WORDS_BIGENDIAN used in installed header avutil.h

[Michael Niedermayer]

On Thu, Jan 29, 2009 at 03:20:10PM -0800, Baptiste Coudurier wrote:
> Michael Niedermayer wrote:
> > On Thu, Jan 29, 2009 at 02:36:53PM -0800, Baptiste Coudurier wrote:
> > [...]
> >>>> considering both values since the
> >>>> goal is to deprecate all _1 (horrible names)
> >>> You are free to suggest better names for the _1 cases, we can surely
> >>> rename them in the next major bump
> >> The best names are RGBA8888, BGRA8888, ARGB8888, ABGR8888, IMHO.
> >> Now RGBA, BGRA, ARGB, ABGR are ok too.
> >>
> >> And this is what should be as external API, there is _no_ sense if
> >> hacking pixfmt externally.
> >>
> >> If libswscale wants to optimize routines based on its internal
> >> implementation (using uint64_t or uint32_t) to deal with data, it's
> >> _its_ problem, not users which should see what is _obvious_.
> > 
> > i dont mind if we move the byte based RGBA, ... into the enum and
> > move the RGB32 stuff to the #defines with the next major bump
> > (it would break ABI not API)
> > assuming someone posts a patch ...
> > 
> > iam not in favor of droping the native 32bit word formats nor do i agree
> > that this is just sws problem, it really is not.
> > a grep in mplayer confirms this also, the 32bit word formats are used
> > the byte based formats are not.
> 
> Well, mplayer is one thing, I know that several people already asked how
> to access raw data from AVFrame.

these people will keep asking no matter what you do with the pixel formats
you arent expecting them to grok the stuff you wrote about rgb15/16 below
do you?
The current stuff is well explained in the header so unless someone fails
to find that he shouldnt have a problem, if he does fail to find it so
will he if something else is writen i there


> 
> If you don't want to use libswscale because it's GPL for example or you
> want to write your own conversion routine or filter, it is definitely
> more simple and obvious to have arch independant representation in
> uint8_t buffer.

Iam not aware of anyone who tried to access rgb15/16 as bytes in a serious
application and succeeded. 
swscale, the old scaler as well as mplayer use 16bit scalers or SIMD

also the c code in sws is 99% LGPL, fixing the rest up shouldnt be that
hard, and the c code should be several times faster than byte based code.


> 
> > also what is your oppinion on the 15/16bit formats, they dont have any
> > sane byte based representation.
> 
> Im not too familiar with them, IMHO we should be consistant that is,
> still considering uint8_t *buf

well we are consistent now, considering everything as native words where
possible.
changing it to uint8_t is to some extend bikeshed.
and fact is all code accesses 15/16 bits using native endian 16bit word
(possibly SIMD) having the internal pixfmt reprsent something that means
something entirely different to code on big and little endian is not
simplifying things at all.

of course if you are just arguing that we should define the enum pixfmts
based on bytes and then have #ifdef WORDS_BIGENDIAN #defines
mapping them to more practical things, i dont really object to that
it is conceptually more correct for example when passing the pixfmt value
over a network ...
but then we have even more things depending on WORDS_BIGENDIAN


> 
> 'R' is (buf[0] >> 3) & 0x1f and 'B' is (buf[1] >> 1) & 0x1f, if RGB5551,
> 'R' is (buf[0] >> 2) & 0x1f and 'B' is buf[1] & 0x1f, if RGB1555

with this system we would need
8 formats for 15bit alone
2x due RGB vs BGR
2x due to big vs. little
2x due to 5551 vs 1555

besides you skiped explaining how green is stored, this is where it becomes
more clear why its not a good idea to use bytes.

currently our docs say
PIX_FMT_BGR555,    ///< packed RGB 5:5:5, 16bpp, (msb)1A 5B 5G 5R(lsb), in CPU endianness, most significant bit to 1

this is clear IMHO while the bit shuffling for bytes is not going to be
pretty

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090130/4d7f69c0/attachment.pgp>