Re: Bug#289182: kino endianness issues on powerpc
Hi,
On Tue, Mar 08, 2005 at 03:39:16PM +0100, Daniel Kobras wrote:
> On Mon, Feb 14, 2005 at 05:41:26PM +0100, Michael Schmitz wrote:
> > I can confirm the XV problem is the same old problem that a patch had
> > been posted for in http://jira.schirmacher.de/jira-kino/browse/KINO-76.
> > I've added some #ifdef __BIG_ENDIAN__ around that, the following patch
> > should finally fix the display issue:
> Err, this patch did fix the display problems for you!? It does
> not touch a single line of code that was executed in the Debian
> build that uses libdv to do the decoding. (Actually, this is no
> longer true as of today. Now with ffmpeg in main, I've
> uploaded a new version that uses libavcodec instead of libdv
> for the decoding part.)
mmh, after testing the patch, it does look much better with :)
> Furthermore, it looks obviously buggy. Eg. the little-endian
> version of the first loop uses values Y[0] and Y[1], while the
> big-endian variant reuses Y[0] twice. And I can't make sense of
> the other array indices, either. I was expecting something like
> dest_big_endian = bswap_32(dest_little_endian); maybe that's
> what was intended, and the current version of the patch makes
> little difference with smooth input data?
well, i am not familiar with these maths, but it does look like
there's some logic:
LE -> BE
Cr[0] << 24 -> Cr[0]
Y [1] << 16 -> Y [0] << 8
Cb[0] << 8 -> Cb[1] << 16
Y [0] -> Y [0] << 24
and for the third line (dest + 4)
Y [2] -> Y [0] << 24
Cb[0] << 8 -> Cb[3] << 16
Y [3] << 16 -> Y [0] << 8
Cr[0] << 24 -> Cr[2]
there is probably a better mean to do so though, ie checking what
type of conversion is needed according to libavcodec, but it does
effectively fixes the XV Display of kino.
> > --- src/frame.cc.org 2005-02-14 16:59:13.798585200 +0100
> > +++ src/frame.cc 2005-02-14 17:14:01.196680184 +0100
attached is and updated patch to go in debian/patches
> I've uploaded kino 0.75-5 that should make the archive by today's
> dinstall run. It includes a comprehensive patch that might fix the
> endianness problems with audio. Alas, I had to do some guessing on the
> endianness of the input data, so it might actually do worse than before,
> but in any case the framework is now in place to fix this with a few
> keypresses.
audio seems to work better now. there are a few glitches but at
least it's not white noise. oh and export to mpeg2 and wav bot
work.
> The second change in 0.75-5 related to this bug was the
> mentioned switch from libdv to libavcodec for video decoding. There's a
> small chance that it fixes the display problem out of the box already.
I can confirm that the switch did not fix anything.
IMO with this last patch the bug should be closed, as the main
functionalities of kino (ie display and export) have been fixed.
there are other bugs around, but they are probably not endian
related nor RC.
Cheers, piem
#! /bin/sh /usr/share/dpatch/dpatch-run
## 40_yuv_endian_fix.dpatch by <piem@altern.org>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: No description.
@DPATCH@
diff -urNad kino-0.75/src/frame.cc /tmp/dpep.RS5WqC/kino-0.75/src/frame.cc
--- kino-0.75/src/frame.cc 2005-03-22 20:29:47.000000000 +0000
+++ /tmp/dpep.RS5WqC/kino-0.75/src/frame.cc 2005-03-22 20:30:25.000000000 +0000
@@ -1052,7 +1052,11 @@
for ( int x = 0; x < width; x += 2 )
{
+#if defined __BIG_ENDIAN__ || defined _BIG_ENDIAN
+ *reinterpret_cast<uint32_t*>( dest ) = Cr[ 0 ] + ( Y[ 0 ] << 8 ) + ( Cb[ 1 ] << 16 ) + ( Y[ 0 ] << 24 );
+#else
*reinterpret_cast<uint32_t*>( dest ) = Y[ 0 ] + ( Cb[ 0 ] << 8 ) + ( Y[ 1 ] << 16 ) + ( Cr[ 0 ] << 24 );
+#endif
dest += 4;
Y += 2;
@@ -1071,8 +1075,13 @@
for ( int x = 0; x < width; x += 4 )
{
+#if defined __BIG_ENDIAN__ || defined _BIG_ENDIAN
+ *reinterpret_cast<uint32_t*>( dest ) = Cr[ 0 ] + ( Y[ 0 ] << 8 ) + ( Cb[ 1 ] << 16 ) + ( Y[ 0 ] << 24 );
+ *reinterpret_cast<uint32_t*>( dest + 4 ) = Cr[ 2 ] + ( Y[ 0 ] << 8 ) + ( Cb[ 3 ] << 16 ) + ( Y[ 0 ] << 24 );
+#else
*reinterpret_cast<uint32_t*>( dest ) = Y[ 0 ] + ( Cb[ 0 ] << 8 ) + ( Y[ 1 ] << 16 ) + ( Cr[ 0 ] << 24 );
*reinterpret_cast<uint32_t*>( dest + 4 ) = Y[ 2 ] + ( Cb[ 0 ] << 8 ) + ( Y[ 3 ] << 16 ) + ( Cr[ 0 ] << 24 );
+#endif
dest += 8;
Y += 4;
Reply to: