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

[Michael Niedermayer]

On Fri, Jan 30, 2009 at 02:40:05AM -0800, Baptiste Coudurier wrote:
> Michael Niedermayer wrote:
> > On Thu, Jan 29, 2009 at 04:24:22PM -0800, Baptiste Coudurier wrote:
> >> Michael Niedermayer wrote:
> >>> 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?
> >> At least it is obvious to me. IMHO it is more obvious than groking
> >> arch dependant description.
> >> 
> >> We all know people are not careful about arch dependant code and 
> >> concepts. I care about usually and simplicity.
> >> 
> >>> 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
> >> Last time I tried to add support for RGBA32 of whatever variant 
> >> quicktime use, I was so confused about these arch dependant things
> >> and these arch dependant defines that I gave up.
> >> 
> >> I hated the guy who decided that buf[0] for RGBA what not 'R' for 
> >> whatever reason.
> > 
> > now we have a problem because buf[0] of PIX_FMT_RGBA is R could you
> > elaborate on what did not work?
> 
> I was trying to figure out what pixfmt to use, according to what my
> buffer contained, according to specs, then I was confused by RGB32_1 or
> RGBA or whatever, however I knew for sure that buf[0] was R, buf[1] was
> G, buf[2] was B, and buf[3] was A.

thats PIX_FMT_RGBA


> 
> >>>> 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
> >>> 
> >> I did try to mess with RGB555 in .mov, and I gave up both in
> >> libswscale and imgconvert because of the arch dependant mess in
> >> pixfmt.
> >> 
> >> We don't agree here.
> >> 
> > [...]
> >> Well, I only see 4 needed pixfmt: RGB1555, RGB5551, BGR5551,
> >> BGR1555.
> > [...]
> >> Well, IMHO this is easy, RGB5551: rrrrrgggggbbbbb1 |   0    |   1
> >> |
> > 
> > the 8 variants, and maybe one is what mov uses (and iam of course not
> > claiming all are used in practice but they could)
> > 
> > RGB5551: rrrrrggg ggbbbbb1 |   0    |   1    |
> > 
> > RGB1555: 1rrrrrgg gggbbbbb |   0    |   1    |
> > 
> > BGR1555: 1bbbbbgg gggrrrrr |   0    |   1    |
> > 
> > BGR5551: bbbbbggg ggrrrrr1 |   0    |   1    |
> > 
> > RGB5551X ggbbbbb1 rrrrrggg |   0    |   1    |
> > 
> > RGB1555X gggbbbbb 1rrrrrgg |   0    |   1    |
> > 
> > BGR1555X gggrrrrr 1bbbbbgg |   0    |   1    |
> > 
> > BGR5551X ggrrrrr1 bbbbbggg |   0    |   1    |
> > 
> > and before you say the X varints dont exist RGB1555X/BGR1555X are the
> > only of the 8 i remember seeing, they are what your *vga card uses on
> > x86 iam not sure of the 5551 variants exist
> 
> Sorry, I don't understand here how you get the X variants, why would
> someone pack values this way, considering you have 3 separate values at
> the beginning, also btw, I don't know much about vga card, nor rgb555
> codecs.

The X variants are byte swaped non X, the names with X being invented by
me due to lack of better a idea (like the _1) they are ugly

Everyone on x86 (little endian) works and worked with RGB555 through
uint16_t or SIMD.
in the cpu register its nice 1RRRRGGGGGBBBBB but when you look at it
byte by byte in memory its due to little endian nature the other way
around namely
GGGBBBBB 1RRRRRGG

Its done that way simply because the oter way around is terribly
inefficient, one would need to byte swap it on little endian.

> 

> I'm confused anyway, why aren't these 8 variants in avutil.h ?

because people only add formats they need and the others seem less
widespread.


> 
> Also I grep for BGR32 in libavcodec, there is no match. Why is this
> pixfmt defined ?

i have matches in rawdec.c, imgconvert and swscale


> 
> What is obvious however is that their name is clearly misleading
> considering what you would have in your uint8_t *buffer.
> 

> Anyway, you still have to find a way to get WORDS_BIGENDIAN defined
> somewhere, this is _critical_ like Diego pointed.

yes
we could have a public config.h and in internal one
the public one could contain things like 
WORDS_BIGENDIAN
FFMPEG_CONFIGURATION
FFMPEG_VERSION

and even things like ENABLE_SMALL, ENABLE_FAST_64BIT, ENABLE_FAST_UNALIGNED
could be very usefull both for static inline functions from libavutil
as well as the user app itself

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/a786603e/attachment.pgp>