Closed GoogleCodeExporter closed 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
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
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
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
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
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
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
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
valgrind issue fixed.
Original comment by fbarch...@chromium.org
on 2 Jul 2014 at 12:31
Original issue reported on code.google.com by
sande...@chromium.org
on 30 May 2014 at 7:43