AshwanthSelvam / libyuv

Automatically exported from code.google.com/p/libyuv
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

I444ToABGR #334

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
libyuv has I420ToABGR and I422ToABGR, but not I444ToABGR; consider adding.

Original issue reported on code.google.com by sande...@chromium.org on 30 May 2014 at 7:43

GoogleCodeExporter commented 9 years ago
The background on this is that libyuv supports I420 and ARGB to and from all 
formats, but not every format to every format.  It can be done as a 2 step 
conversion.
I422 and I420 use the same low level function, so I422 is able to convert to 
most formats as well.
I444ToARGB is supported.  ABGR is the least common of the RGB formats.

Do you have a need for this I444ToABGR function?
If so its feasible to do.
If its not an actual requirement, a longer term solution would be a conversion 
high level function that converts anything to anything, using multiple 
conversion steps if necessary.

Original comment by fbarch...@chromium.org on 2 Jun 2014 at 6:19

GoogleCodeExporter commented 9 years ago
I wouldn't say that I need it, because as you say it can be done in multiple 
steps (at the cost of an extra allocation in my case), and in my case the code 
path is unlikely to be used often. It's just unfortunate that that extra 
allocation applies specifically to Android (where Skia uses ABGR).

I'd actually be just as happy if I could do ARGB->ABGR efficiently in-place.

Original comment by sande...@chromium.org on 2 Jun 2014 at 6:28

GoogleCodeExporter commented 9 years ago
ARGB to/from all RGB formats should work in place.
The code was written to read the pixels before doing any writes so you can do 
that.

The function you're looking for is actually ABGRToARGB which is the same as 
ARGBToABGR.
But its simply a macro.  To help clarify the function exists I'll make it a 
proper function.

Original comment by fbarch...@chromium.org on 2 Jun 2014 at 7:01

GoogleCodeExporter commented 9 years ago
r1008 reimplements ARGBToABGR as a regular function instead of a macro.
It can be used in-place.

Original comment by fbarch...@chromium.org on 2 Jun 2014 at 7:25

GoogleCodeExporter commented 9 years ago
r1008 is approved and rolling into webrtc and chromium.

Will that suffice or would you still like to see I444ToABGR in one step?

Original comment by fbarch...@chromium.org on 2 Jun 2014 at 10:34

GoogleCodeExporter commented 9 years ago
That's good with me, I'm happy now that I know ABGRToARGB is guaranteed to 
handle aliasing correctly.

Original comment by sande...@chromium.org on 2 Jun 2014 at 10:41

GoogleCodeExporter commented 9 years ago
In r1009 I've added a mediocre unittest that does an inplace ABGRToARGB and 
checks that when done twice the result is back to the original.  There are only 
3 formats that are symmetric so not the best test, but it will ensure that the 
compiler doesnt try to reorder the instructions into an unsafe read/write 
pattern.

I may follow up with a better test which could do most formats on a plane by 
plane level, in place.  e.g. I444 to I420 can be done in place, as can 
RGB24ToARGB.

Original comment by fbarch...@chromium.org on 4 Jun 2014 at 11:20

GoogleCodeExporter commented 9 years ago
valgrind doesnt like memcpy used in place:

16:24:39 memcheck_analyze.py [ERROR] Command: src/out/Release/libyuv_unittest
Overlap
Source and destination overlap in memcpy(0x706f500, 0x706f500, 35712)
  memcpy@@GLIBC_2.14 (/tmp/valgrind-src/valgrind-memcheck/valgrind/memcheck/mc_replace_strmem.c:877)
  CopyRow_C (/usr/include/x86_64-linux-gnu/bits/string3.h:52)
  CopyPlane (source/planar_functions.cc:70)
  ARGBCopy (source/convert_argb.cc:44)
  libyuv::libyuvTest_ARGBCopy_Symetric_Any_Test::TestBody() (/mnt/data/b/build/slave/Linux_Memcheck/build/src/out/Release/libyuv_unittest)
Suppression (error hash=#F05AD3C7E000C83C#):
  For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports
{
   <insert_a_suppression_name_here>
   Memcheck:Overlap
   fun:memcpy@@GLIBC_2.14
   fun:CopyRow_C
   fun:CopyPlane
   fun:ARGBCopy
   fun:_ZN6libyuv37libyuvTest_ARGBCopy_Symetric_Any_Test8TestBodyEv
}

16:24:39 valgrind_test.py [INFO] Please see 
http://dev.chromium.org/developers/how-tos/using-valgrind for the info on 
Memcheck/Valgrind
16:24:39 valgrind_test.py [ERROR] Analyze failed.
16:24:39 valgrind_test.py [INFO] Search the log for '[ERROR]' to see the error 
reports.
16:24:39 valgrind_test.py [INFO] elapsed time: 00:01:19
Stopping Xvfb with pid 25572 ...
Xvfb pid file removed
exit code (as seen by runtest.py): 255
@@@STEP_LOG_LINE@F05AD3C7E000C83C@Suppression (error 
hash=#F05AD3C7E000C83C#):@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@For more info on using suppressions see 
http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory
-sheriff#TOC-Suppressing-memory-reports@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@{@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@<insert_a_suppression_name_here>@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@Memcheck:Overlap@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@fun:memcpy@@GLIBC_2.14@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@fun:CopyRow_C@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@fun:CopyPlane@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@fun:ARGBCopy@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@fun:_ZN6libyuv37libyuvTest_ARGBCopy_Symetric_A
ny_Test8TestBodyEv@@@
@@@STEP_LOG_LINE@F05AD3C7E000C83C@}@@@
@@@STEP_LOG_END@F05AD3C7E000C83C@@@
@@@STEP_FAILURE@@@
@@@STEP_TEXT@memory test: libyuv_unittest@@@
@@@STEP_TEXT@1 disabled@@@
@@@STEP_TEXT@crashed or hung@@@
 killed dbus-daemon with PID 25571
 cleared DBUS_SESSION_BUS_ADDRESS environment variable
program finished with exit code 255
elapsedTime=79.713148

Original comment by fbarch...@chromium.org on 5 Jun 2014 at 6:01

GoogleCodeExporter commented 9 years ago
valgrind issue fixed.

Original comment by fbarch...@chromium.org on 2 Jul 2014 at 12:31