[RFC] Make swscale-test perform only one convertion
Hi all, this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way. I expected the new result to be different from the old one as the random source is different, but I noted that the new result is almost always *worse* than the old one (that is the output obtained with the old swscale-test), can someone explain? Here it follows a sample of diff -u swscale.test.oldres swscale.test.newres: --- swscale.test.ref 2010-01-23 00:50:06.000000000 +0100 +++ swscale.test.ref.new 2010-01-29 01:32:51.000000000 +0100 @@ -90,19 +90,19 @@ yuv420p 96x96 -> yuyv422 96x 128 flags= 8 SSD= 0, 0, 0, 0 yuv420p 96x96 -> yuyv422 96x 128 flags=16 SSD= 0, 0, 0, 0 yuv420p 96x96 -> yuyv422 96x 128 flags=32 SSD= 0, 0, 0, 0 - yuv420p 96x96 -> yuyv422 128x 64 flags= 1 SSD= 1, 4, 6, 0 + yuv420p 96x96 -> yuyv422 128x 64 flags= 1 SSD= 1, 5, 9, 0 yuv420p 96x96 -> yuyv422 128x 64 flags= 2 SSD= 0, 0, 0, 0 yuv420p 96x96 -> yuyv422 128x 64 flags= 4 SSD= 0, 0, 0, 0 yuv420p 96x96 -> yuyv422 128x 64 flags= 8 SSD= 0, 1, 1, 0 yuv420p 96x96 -> yuyv422 128x 64 flags=16 SSD= 6, 0, 0, 0 yuv420p 96x96 -> yuyv422 128x 64 flags=32 SSD= 0, 1, 1, 0 - yuv420p 96x96 -> yuyv422 128x 96 flags= 1 SSD= 2, 8, 12, 0 + yuv420p 96x96 -> yuyv422 128x 96 flags= 1 SSD= 2, 9, 12, 0 yuv420p 96x96 -> yuyv422 128x 96 flags= 2 SSD= 0, 5, 3, 0 yuv420p 96x96 -> yuyv422 128x 96 flags= 4 SSD= 0, 0, 0, 0 yuv420p 96x96 -> yuyv422 128x 96 flags= 8 SSD= 0, 5, 3, 0 yuv420p 96x96 -> yuyv422 128x 96 flags=16 SSD= 0, 0, 0, 0 yuv420p 96x96 -> yuyv422 128x 96 flags=32 SSD= 0, 5, 4, 0 - yuv420p 96x96 -> yuyv422 128x 128 flags= 1 SSD= 1, 6, 9, 0 + yuv420p 96x96 -> yuyv422 128x 128 flags= 1 SSD= 1, 5, 9, 0 yuv420p 96x96 -> yuyv422 128x 128 flags= 2 SSD= 0, 1, 1, 0 yuv420p 96x96 -> yuyv422 128x 128 flags= 4 SSD= 0, 0, 0, 0 yuv420p 96x96 -> yuyv422 128x 128 flags= 8 SSD= 0, 1, 1, 0 @@ -145,19 +145,19 @@ yuv420p 96x96 -> rgb24 96x 128 flags= 8 SSD= 3, 0, 1, 0 yuv420p 96x96 -> rgb24 96x 128 flags=16 SSD= 1, 0, 0, 0 yuv420p 96x96 -> rgb24 96x 128 flags=32 SSD= 3, 0, 0, 0 - yuv420p 96x96 -> rgb24 128x 64 flags= 1 SSD= 4, 2, 5, 0 + yuv420p 96x96 -> rgb24 128x 64 flags= 1 SSD= 4, 3, 8, 0 yuv420p 96x96 -> rgb24 128x 64 flags= 2 SSD= 1, 0, 0, 0 yuv420p 96x96 -> rgb24 128x 64 flags= 4 SSD= 1, 0, 0, 0 yuv420p 96x96 -> rgb24 128x 64 flags= 8 SSD= 2, 1, 1, 0 yuv420p 96x96 -> rgb24 128x 64 flags=16 SSD= 7, 0, 0, 0 yuv420p 96x96 -> rgb24 128x 64 flags=32 SSD= 3, 1, 1, 0 - yuv420p 96x96 -> rgb24 128x 96 flags= 1 SSD= 3, 7, 10, 0 + yuv420p 96x96 -> rgb24 128x 96 flags= 1 SSD= 3, 13, 12, 0 yuv420p 96x96 -> rgb24 128x 96 flags= 2 SSD= 2, 5, 3, 0 yuv420p 96x96 -> rgb24 128x 96 flags= 4 SSD= 1, 0, 0, 0 yuv420p 96x96 -> rgb24 128x 96 flags= 8 SSD= 2, 5, 4, 0 yuv420p 96x96 -> rgb24 128x 96 flags=16 SSD= 1, 0, 0, 0 yuv420p 96x96 -> rgb24 128x 96 flags=32 SSD= 2, 6, 4, 0 - yuv420p 96x96 -> rgb24 128x 128 flags= 1 SSD= 4, 3, 9, 0 + yuv420p 96x96 -> rgb24 128x 128 flags= 1 SSD= 4, 5, 9, 0 yuv420p 96x96 -> rgb24 128x 128 flags= 2 SSD= 3, 1, 1, 0 yuv420p 96x96 -> rgb24 128x 128 flags= 4 SSD= 1, 0, 0, 0 yuv420p 96x96 -> rgb24 128x 128 flags= 8 SSD= 3, 1, 1, 0 @@ -200,38 +200,38 @@ yuv420p 96x96 -> bgr24 96x 128 flags= 8 SSD= 3, 0, 1, 0 yuv420p 96x96 -> bgr24 96x 128 flags=16 SSD= 1, 0, 0, 0 yuv420p 96x96 -> bgr24 96x 128 flags=32 SSD= 3, 0, 0, 0 - yuv420p 96x96 -> bgr24 128x 64 flags= 1 SSD= 4, 2, 5, 0 + yuv420p 96x96 -> bgr24 128x 64 flags= 1 SSD= 4, 3, 8, 0 yuv420p 96x96 -> bgr24 128x 64 flags= 2 SSD= 1, 0, 0, 0 yuv420p 96x96 -> bgr24 128x 64 flags= 4 SSD= 1, 0, 0, 0 yuv420p 96x96 -> bgr24 128x 64 flags= 8 SSD= 2, 1, 1, 0 yuv420p 96x96 -> bgr24 128x 64 flags=16 SSD= 7, 0, 0, 0 yuv420p 96x96 -> bgr24 128x 64 flags=32 SSD= 3, 1, 1, 0 [...] rgb24 -> yuv411p - rgb24 96x96 -> yuv411p 64x 64 flags= 1 SSD= 3, 28, 22, 0 + rgb24 96x96 -> yuv411p 64x 64 flags= 1 SSD= 16, 84, 167, 0 rgb24 96x96 -> yuv411p 64x 64 flags= 2 SSD= 2, 12, 15, 0 rgb24 96x96 -> yuv411p 64x 64 flags= 4 SSD= 1, 2, 3, 0 rgb24 96x96 -> yuv411p 64x 64 flags= 8 SSD= 2, 13, 15, 0 rgb24 96x96 -> yuv411p 64x 64 flags=16 SSD= 13, 29, 31, 0 rgb24 96x96 -> yuv411p 64x 64 flags=32 SSD= 2, 10, 13, 0 - rgb24 96x96 -> yuv411p 64x 96 flags= 1 SSD= 4, 33, 101, 0 + rgb24 96x96 -> yuv411p 64x 96 flags= 1 SSD= 11, 68, 154, 0 rgb24 96x96 -> yuv411p 64x 96 flags= 2 SSD= 1, 12, 15, 0 rgb24 96x96 -> yuv411p 64x 96 flags= 4 SSD= 1, 2, 3, 0 rgb24 96x96 -> yuv411p 64x 96 flags= 8 SSD= 2, 12, 15, 0 rgb24 96x96 -> yuv411p 64x 96 flags=16 SSD= 6, 29, 31, 0 rgb24 96x96 -> yuv411p 64x 96 flags=32 SSD= 1, 9, 12, 0 - rgb24 96x96 -> yuv411p 64x 128 flags= 1 SSD= 4, 33, 166, 0 + rgb24 96x96 -> yuv411p 64x 128 flags= 1 SSD= 8, 89, 166, 0 rgb24 96x96 -> yuv411p 64x 128 flags= 2 SSD= 1, 12, 15, 0 rgb24 96x96 -> yuv411p 64x 128 flags= 4 SSD= 1, 2, 3, 0 rgb24 96x96 -> yuv411p 64x 128 flags= 8 SSD= 2, 12, 15, 0 rgb24 96x96 -> yuv411p 64x 128 flags=16 SSD= 6, 29, 31, 0 rgb24 96x96 -> yuv411p 64x 128 flags=32 SSD= 1, 9, 12, 0 - rgb24 96x96 -> yuv411p 96x 64 flags= 1 SSD= 1, 21, 46, 0 + rgb24 96x96 -> yuv411p 96x 64 flags= 1 SSD= 1, 26, 76, 0 rgb24 96x96 -> yuv411p 96x 64 flags= 2 SSD= 1, 4, 5, 0 rgb24 96x96 -> yuv411p 96x 64 flags= 4 SSD= 1, 0, 0, 0 rgb24 96x96 -> yuv411p 96x 64 flags= 8 SSD= 2, 4, 4, 0 rgb24 96x96 -> yuv411p 96x 64 flags=16 SSD= 7, 18, 17, 0 rgb24 96x96 -> yuv411p 96x 64 flags=32 SSD= 1, 2, 3, 0 - rgb24 96x96 -> yuv411p 96x 96 flags= 1 SSD= 1, 11, 74, 0 + rgb24 96x96 -> yuv411p 96x 96 flags= 1 SSD= 1, 47, 74, 0 rgb24 96x96 -> yuv411p 96x 96 flags= 2 SSD= 1, 4, 4, 0 rgb24 96x96 -> yuv411p 96x 96 flags= 4 SSD= 1, 0, 0, 0 rgb24 96x96 -> yuv411p 96x 96 flags= 8 SSD= 1, 3, 3, 0 rgb24 96x96 -> yuv411p 96x 96 flags=16 SSD= 1, 18, 17, 0 rgb24 96x96 -> yuv411p 96x 96 flags=32 SSD= 1, 1, 2, 0 - rgb24 96x96 -> yuv411p 96x 128 flags= 1 SSD= 1, 13, 74, 0 + rgb24 96x96 -> yuv411p 96x 128 flags= 1 SSD= 1, 45, 74, 0 rgb24 96x96 -> yuv411p 96x 128 flags= 2 SSD= 1, 4, 4, 0 rgb24 96x96 -> yuv411p 96x 128 flags= 4 SSD= 1, 0, 0, 0 rgb24 96x96 -> yuv411p 96x 128 flags= 8 SSD= 1, 3, 3, 0 rgb24 96x96 -> yuv411p 96x 128 flags=16 SSD= 1, 18, 17, 0 rgb24 96x96 -> yuv411p 96x 128 flags=32 SSD= 1, 1, 2, 0 - rgb24 96x96 -> yuv411p 128x 64 flags= 1 SSD= 5, 8, 34, 0 + rgb24 96x96 -> yuv411p 128x 64 flags= 1 SSD= 5, 19, 34, 0 rgb24 96x96 -> yuv411p 128x 64 flags= 2 SSD= 1, 2, 2, 0 rgb24 96x96 -> yuv411p 128x 64 flags= 4 SSD= 1, 0, 0, 0 rgb24 96x96 -> yuv411p 128x 64 flags= 8 SSD= 2, 2, 2, 0 rgb24 96x96 -> yuv411p 128x 64 flags=16 SSD= 7, 14, 15, 0 rgb24 96x96 -> yuv411p 128x 64 flags=32 SSD= 1, 2, 2, 0 - rgb24 96x96 -> yuv411p 128x 96 flags= 1 SSD= 5, 5, 33, 0 + rgb24 96x96 -> yuv411p 128x 96 flags= 1 SSD= 5, 32, 33, 0 rgb24 96x96 -> yuv411p 128x 96 flags= 2 SSD= 1, 1, 1, 0 rgb24 96x96 -> yuv411p 128x 96 flags= 4 SSD= 1, 0, 0, 0 rgb24 96x96 -> yuv411p 128x 96 flags= 8 SSD= 1, 2, 2, 0 rgb24 96x96 -> yuv411p 128x 96 flags=16 SSD= 1, 14, 15, 0 rgb24 96x96 -> yuv411p 128x 96 flags=32 SSD= 1, 1, 1, 0 - rgb24 96x96 -> yuv411p 128x 128 flags= 1 SSD= 5, 17, 33, 0 + rgb24 96x96 -> yuv411p 128x 128 flags= 1 SSD= 5, 35, 33, 0 Also note that I don't want to hardcode the supported in/out formats in the swscale-test-all.sh script, but currently I don't know which is the best way to do that, I cannot extend avcodec_pix_fmt_string() with sws_isSupported{In,Out} since that would add a dependancy lsws <- lavc, my idea was to define a get_pix_fmt_string() in cmdutils.c. Suggestions are welcome. Regards. -- FFmpeg = Furious & Friendly Mystic Porno Esoteric Guide
On Thu, Jan 28, 2010 at 10:52 PM, Stefano Sabatini <stefano.sabatini-lala at poste.it> wrote:
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
With my swscale-test it's even easier =)
I expected the new result to be different from the old one as the random source is different, but I noted that the new result is almost always *worse* than the old one (that is the output obtained with the old swscale-test), can someone explain?
I don't understand. If it's random, it should be better or worse randomly (or the same), you shouldn't try to predict anything in the change.
Stefano Sabatini wrote:
Hi all,
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
I expected the new result to be different from the old one as the random source is different,
This is easy to work-around, just re-init LFG fo every format pair in the unpatched swscale-test and it should give the same results.
but I noted that the new result is almost always *worse* than the old one (that is the output obtained with the old swscale-test), can someone explain?
Have you tried running twice with exactly the same binary to see if there is nothing reading uninitialized mem? -Vitor
On Fri, Jan 29, 2010 at 2:52 AM, Vitor Sessak <vitor1001 at gmail.com> wrote:
Stefano Sabatini wrote:
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
I expected the new result to be different from the old one as the random source is different,
This is easy to work-around, just re-init LFG fo every format pair in the unpatched swscale-test and it should give the same results.
but I noted that the new result is almost always *worse* than the old one (that is the output obtained with the old swscale-test), can someone explain?
Have you tried running twice with exactly the same binary to see if there is nothing reading uninitialized mem?
The fast bilinear code reads unitialized memory (it always overreads at least one pixel).
On date Friday 2010-01-29 06:16:14 -0200, Ramiro Polla encoded:
On Fri, Jan 29, 2010 at 2:52 AM, Vitor Sessak <vitor1001 at gmail.com> wrote:
Stefano Sabatini wrote:
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
I expected the new result to be different from the old one as the random source is different,
This is easy to work-around, just re-init LFG fo every format pair in the unpatched swscale-test and it should give the same results.
but I noted that the new result is almost always *worse* than the old one (that is the output obtained with the old swscale-test), can someone explain?
Have you tried running twice with exactly the same binary to see if there is nothing reading uninitialized mem?
The fast bilinear code reads unitialized memory (it always overreads at least one pixel).
This patch makes the swscale-test output constant, with both the current swscale-test and and my patched swscale-test+swscale-test-all.sh. Regards. -- FFmpeg = Fanciful and Frenzy Most Pure Elegant Gigant
On Sat, Jan 30, 2010 at 01:40:29PM +0100, Stefano Sabatini wrote:
On date Friday 2010-01-29 06:16:14 -0200, Ramiro Polla encoded:
On Fri, Jan 29, 2010 at 2:52 AM, Vitor Sessak <vitor1001 at gmail.com> wrote:
Stefano Sabatini wrote:
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
I expected the new result to be different from the old one as the random source is different,
This is easy to work-around, just re-init LFG fo every format pair in the unpatched swscale-test and it should give the same results.
but I noted that the new result is almost always *worse* than the old one (that is the output obtained with the old swscale-test), can someone explain?
Have you tried running twice with exactly the same binary to see if there is nothing reading uninitialized mem?
The fast bilinear code reads unitialized memory (it always overreads at least one pixel).
This patch makes the swscale-test output constant, with both the current swscale-test and and my patched swscale-test+swscale-test-all.sh.
Regards. -- FFmpeg = Fanciful and Frenzy Most Pure Elegant Gigant
swscale-test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 5015d3a1a95f9408dc59e6d7a47302c795e7734a fix-swscale-test-randomness.patch
ok [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato
On date Saturday 2010-01-30 13:49:05 +0100, Michael Niedermayer encoded:
On Sat, Jan 30, 2010 at 01:40:29PM +0100, Stefano Sabatini wrote: [...]
This patch makes the swscale-test output constant, with both the current swscale-test and and my patched swscale-test+swscale-test-all.sh.
Regards. -- FFmpeg = Fanciful and Frenzy Most Pure Elegant Gigant
swscale-test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 5015d3a1a95f9408dc59e6d7a47302c795e7734a fix-swscale-test-randomness.patch
ok
Applied. -- FFmpeg = Friendly and Fundamentalist Minimalistic Pacific Enhancing Game
On date Friday 2010-01-29 01:52:23 +0100, Stefano Sabatini encoded:
Hi all,
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
Updated. -- FFmpeg = Fundamentalist and Fancy Minimalistic Portable Ermetic Game
On Fri, Feb 12, 2010 at 12:32:44AM +0100, Stefano Sabatini wrote: [...]
+ if (((i = 1) && ((srcFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE)) || + ((i = 2) && ((dstFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE))) { + fprintf(stderr, "Unknown pixel format name: '%s'\n", argv[i]); + return -1; + }
wrong mailing list, this is not the obfuscated code contest [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes
On date Friday 2010-02-12 01:30:23 +0100, Michael Niedermayer encoded:
On Fri, Feb 12, 2010 at 12:32:44AM +0100, Stefano Sabatini wrote: [...]
+ if (((i = 1) && ((srcFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE)) || + ((i = 2) && ((dstFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE))) { + fprintf(stderr, "Unknown pixel format name: '%s'\n", argv[i]); + return -1; + }
wrong mailing list, this is not the obfuscated code contest
Uh, maybe I overdid at trying to keep low the line count... Regards. -- FFmpeg = Fancy & Furious Moronic Pacific Extravagant Geek
On Tue, Feb 16, 2010 at 01:29:02AM +0100, Stefano Sabatini wrote:
On date Friday 2010-02-12 01:30:23 +0100, Michael Niedermayer encoded:
On Fri, Feb 12, 2010 at 12:32:44AM +0100, Stefano Sabatini wrote: [...]
+ if (((i = 1) && ((srcFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE)) || + ((i = 2) && ((dstFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE))) { + fprintf(stderr, "Unknown pixel format name: '%s'\n", argv[i]); + return -1; + }
wrong mailing list, this is not the obfuscated code contest
Uh, maybe I overdid at trying to keep low the line count...
Regards. -- FFmpeg = Fancy & Furious Moronic Pacific Extravagant Geek
swscale-test-all.sh | 11 +++++++++++ swscale-test.c | 36 +++++++++++++++++++----------------- 2 files changed, 30 insertions(+), 17 deletions(-) 1766e975f79d21a0b4c898fe8f41c4d2f1e8a615 implement-swscale-test-lite.patch
I think we should wait until we have a benchmark on mingw or reduce the amount of shell work aka use C instead [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes
On date Tuesday 2010-02-16 20:27:33 +0100, Michael Niedermayer encoded:
On Tue, Feb 16, 2010 at 01:29:02AM +0100, Stefano Sabatini wrote:
On date Friday 2010-02-12 01:30:23 +0100, Michael Niedermayer encoded:
On Fri, Feb 12, 2010 at 12:32:44AM +0100, Stefano Sabatini wrote: [...]
+ if (((i = 1) && ((srcFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE)) || + ((i = 2) && ((dstFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE))) { + fprintf(stderr, "Unknown pixel format name: '%s'\n", argv[i]); + return -1; + }
wrong mailing list, this is not the obfuscated code contest
Uh, maybe I overdid at trying to keep low the line count...
Regards. -- FFmpeg = Fancy & Furious Moronic Pacific Extravagant Geek
swscale-test-all.sh | 11 +++++++++++ swscale-test.c | 36 +++++++++++++++++++----------------- 2 files changed, 30 insertions(+), 17 deletions(-) 1766e975f79d21a0b4c898fe8f41c4d2f1e8a615 implement-swscale-test-lite.patch
I think we should wait until we have a benchmark on mingw or reduce the amount of shell work aka use C instead
Ramiro can you provide a benchmark on MinGW? Also my gut feeling is that we should use shell when meaningful and make pression to get things improved at the root of the problem (which is MinGW in this case). Regards. -- FFmpeg = Fundamentalist and Fostering Mournful Pure Erratic Gorilla
On Wed, Feb 17, 2010 at 9:52 PM, Stefano Sabatini <stefano.sabatini-lala at poste.it> wrote:
On date Tuesday 2010-02-16 20:27:33 +0100, Michael Niedermayer encoded:
On Tue, Feb 16, 2010 at 01:29:02AM +0100, Stefano Sabatini wrote:
On date Friday 2010-02-12 01:30:23 +0100, Michael Niedermayer encoded:
On Fri, Feb 12, 2010 at 12:32:44AM +0100, Stefano Sabatini wrote: [...]
+ ? ?if (((i = 1) && ((srcFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE)) || + ? ? ? ?((i = 2) && ((dstFmt = av_get_pix_fmt(argv[i])) == PIX_FMT_NONE))) { + ? ? ? ?fprintf(stderr, "Unknown pixel format name: '%s'\n", argv[i]); + ? ? ? ?return -1; + ? ?}
wrong mailing list, this is not the obfuscated code contest
Uh, maybe I overdid at trying to keep low the line count...
Regards. -- FFmpeg = Fancy & Furious Moronic Pacific Extravagant Geek
?swscale-test-all.sh | ? 11 +++++++++++ ?swscale-test.c ? ? ?| ? 36 +++++++++++++++++++----------------- ?2 files changed, 30 insertions(+), 17 deletions(-) 1766e975f79d21a0b4c898fe8f41c4d2f1e8a615 ?implement-swscale-test-lite.patch
I think we should wait until we have a benchmark on mingw or reduce the amount of shell work aka use C instead
Ramiro can you provide a benchmark on MinGW?
swscale-test is currently crashing in Windows in ./swscale-test.exe rgb24 argb. Apparently due to heap corruption (that's what windows says), but I couldn't find out the exact cause yet.
Also my gut feeling is that we should use shell when meaningful and make pression to get things improved at the root of the problem (which is MinGW in this case).
The problem is windows slow at launching processes. By the way is this intended to work out-of-tree? Ramiro Polla
On Thu, Feb 18, 2010 at 2:25 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
swscale-test is currently crashing in Windows in ./swscale-test.exe rgb24 argb. Apparently due to heap corruption (that's what windows says), but I couldn't find out the exact cause yet.
It crashes on: rgb24 96x96 -> argb 96x 96 flags=16 While trying to free dst in swscale-test.c:doTest() for (i=0; i<4; i++) { av_free(src[i]); av_free(dst[i]); av_free(out[i]); } The function rgb24tobgr32 has overwritten data past the end of its destination pointer because swscale.c:rgbToRgbWrapper() checks: if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) dstPtr += ALT32_CORR; and increments dstPtr. Stefano, I see that you have added that !isRGBA32() there. Could you double check if this is correct for all cases? Shouldn't it actually be: Index: swscale.c =================================================================== --- swscale.c (revision 30631) +++ swscale.c (working copy) @@ -1502,10 +1502,10 @@ } else { const uint8_t *srcPtr= src[0]; uint8_t *dstPtr= dst[0]; - if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) + if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) srcPtr += ALT32_CORR; - if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) + if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) dstPtr += ALT32_CORR; if (dstStride[0]*srcBpp == srcStride[0]*dstBpp && srcStride[0] > 0)
On date Thursday 2010-02-18 04:10:44 -0200, Ramiro Polla encoded:
On Thu, Feb 18, 2010 at 2:25 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
swscale-test is currently crashing in Windows in ./swscale-test.exe rgb24 argb. Apparently due to heap corruption (that's what windows says), but I couldn't find out the exact cause yet.
It crashes on: rgb24 96x96 -> argb 96x 96 flags=16
While trying to free dst in swscale-test.c:doTest() for (i=0; i<4; i++) { av_free(src[i]); av_free(dst[i]); av_free(out[i]); }
The function rgb24tobgr32 has overwritten data past the end of its destination pointer because swscale.c:rgbToRgbWrapper() checks: if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) dstPtr += ALT32_CORR; and increments dstPtr.
Stefano, I see that you have added that !isRGBA32() there. Could you double check if this is correct for all cases? Shouldn't it actually be:
Index: swscale.c =================================================================== --- swscale.c (revision 30631) +++ swscale.c (working copy) @@ -1502,10 +1502,10 @@ } else { const uint8_t *srcPtr= src[0]; uint8_t *dstPtr= dst[0]; - if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) + if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) srcPtr += ALT32_CORR;
- if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) + if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) dstPtr += ALT32_CORR;
if (dstStride[0]*srcBpp == srcStride[0]*dstBpp && srcStride[0] > 0)
No this is wrong. But I see there is a problem here, we have rgb24to32 writing over the end of the slice. Maybe we need to implement some specific function for dealing with that case (BTW we are also skipping to write the first byte of the ARGB array). Regards. -- FFmpeg = Funny and Frightening Magical Pure EniGma
On date Thursday 2010-02-18 09:29:19 +0100, Stefano Sabatini encoded:
On date Thursday 2010-02-18 04:10:44 -0200, Ramiro Polla encoded:
On Thu, Feb 18, 2010 at 2:25 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
swscale-test is currently crashing in Windows in ./swscale-test.exe rgb24 argb. Apparently due to heap corruption (that's what windows says), but I couldn't find out the exact cause yet.
It crashes on: rgb24 96x96 -> argb 96x 96 flags=16
While trying to free dst in swscale-test.c:doTest() for (i=0; i<4; i++) { av_free(src[i]); av_free(dst[i]); av_free(out[i]); }
The function rgb24tobgr32 has overwritten data past the end of its destination pointer because swscale.c:rgbToRgbWrapper() checks: if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) dstPtr += ALT32_CORR; and increments dstPtr.
Stefano, I see that you have added that !isRGBA32() there. Could you double check if this is correct for all cases? Shouldn't it actually be:
Index: swscale.c =================================================================== --- swscale.c (revision 30631) +++ swscale.c (working copy) @@ -1502,10 +1502,10 @@ } else { const uint8_t *srcPtr= src[0]; uint8_t *dstPtr= dst[0]; - if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) + if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) srcPtr += ALT32_CORR;
- if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) + if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) dstPtr += ALT32_CORR;
if (dstStride[0]*srcBpp == srcStride[0]*dstBpp && srcStride[0] > 0)
No this is wrong.
But I see there is a problem here, we have rgb24to32 writing over the end of the slice. Maybe we need to implement some specific function for dealing with that case (BTW we are also skipping to write the first byte of the ARGB array).
Please try the attached patch. BTW we should find some way to bypass the rgbToRgbWrapper and cache conv function once and for all. Regards. -- FFmpeg = Fostering Fascinating Muttering Peaceful Everlasting Guru
On Fri, Feb 19, 2010 at 12:23:36AM +0100, Stefano Sabatini wrote:
On date Thursday 2010-02-18 09:29:19 +0100, Stefano Sabatini encoded:
On date Thursday 2010-02-18 04:10:44 -0200, Ramiro Polla encoded:
On Thu, Feb 18, 2010 at 2:25 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
swscale-test is currently crashing in Windows in ./swscale-test.exe rgb24 argb. Apparently due to heap corruption (that's what windows says), but I couldn't find out the exact cause yet.
It crashes on: rgb24 96x96 -> argb 96x 96 flags=16
While trying to free dst in swscale-test.c:doTest() for (i=0; i<4; i++) { av_free(src[i]); av_free(dst[i]); av_free(out[i]); }
The function rgb24tobgr32 has overwritten data past the end of its destination pointer because swscale.c:rgbToRgbWrapper() checks: if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) dstPtr += ALT32_CORR; and increments dstPtr.
Stefano, I see that you have added that !isRGBA32() there. Could you double check if this is correct for all cases? Shouldn't it actually be:
Index: swscale.c =================================================================== --- swscale.c (revision 30631) +++ swscale.c (working copy) @@ -1502,10 +1502,10 @@ } else { const uint8_t *srcPtr= src[0]; uint8_t *dstPtr= dst[0]; - if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) + if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) srcPtr += ALT32_CORR;
- if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) + if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) dstPtr += ALT32_CORR;
if (dstStride[0]*srcBpp == srcStride[0]*dstBpp && srcStride[0] > 0)
No this is wrong.
But I see there is a problem here, we have rgb24to32 writing over the end of the slice. Maybe we need to implement some specific function for dealing with that case (BTW we are also skipping to write the first byte of the ARGB array).
Please try the attached patch.
BTW we should find some way to bypass the rgbToRgbWrapper and cache conv function once and for all.
Regards. -- FFmpeg = Fostering Fascinating Muttering Peaceful Everlasting Guru
rgb2rgb.c | 31 +++++++++++++++++++++++++++++++ rgb2rgb.h | 5 +++++ swscale.c | 23 +++++++++++++++++++---- 3 files changed, 55 insertions(+), 4 deletions(-) 42d7b55826f1b1a497f1f6c17f374832ee1579ec fix-rgb24-to32-conv.patch Index: ffmpeg/libswscale/rgb2rgb.c =================================================================== --- ffmpeg.orig/libswscale/rgb2rgb.c 2010-02-18 23:04:47.000000000 +0100 +++ ffmpeg/libswscale/rgb2rgb.c 2010-02-19 00:16:02.000000000 +0100 @@ -462,3 +462,34 @@ DEFINE_SHUFFLE_BYTES(3, 0, 1, 2); DEFINE_SHUFFLE_BYTES(3, 2, 1, 0);
+#define DEFINE_SHUFFLE_BYTES_ABCX(a, b, c) \ +void shuffle_bytes_##a##b##c##X(const uint8_t *src, uint8_t *dst, long src_size) \ +{ \ + long i, j; \ + \ + for (i = 0, j = 0; i < src_size; i+=3, j+=4) { \ + dst[j + 0] = src[i + a]; \ + dst[j + 1] = src[i + b]; \ + dst[j + 2] = src[i + c]; \ + dst[j + 3] = 255; \ + } \ +} + +DEFINE_SHUFFLE_BYTES_ABCX(0, 1, 2); +DEFINE_SHUFFLE_BYTES_ABCX(2, 1, 0); +
+#define DEFINE_SHUFFLE_BYTES_XABC(a, b, c) \ +void shuffle_bytes_X##a##b##c(const uint8_t *src, uint8_t *dst, long src_size) \ +{ \ + long i, j; \ + \ + for (i = 0, j = 0; i < src_size; i+=3, j+=4) { \ + dst[j + 0] = 255; \ + dst[j + 1] = src[i + a]; \ + dst[j + 2] = src[i + b]; \ + dst[j + 3] = src[i + c]; \ + } \ +} + +DEFINE_SHUFFLE_BYTES_XABC(0, 1, 2); +DEFINE_SHUFFLE_BYTES_XABC(2, 1, 0); Index: ffmpeg/libswscale/rgb2rgb.h =================================================================== --- ffmpeg.orig/libswscale/rgb2rgb.h 2010-02-18 23:18:53.000000000 +0100 +++ ffmpeg/libswscale/rgb2rgb.h 2010-02-18 23:34:41.000000000 +0100 @@ -66,6 +66,11 @@ void shuffle_bytes_3012(const uint8_t *src, uint8_t *dst, long src_size); void shuffle_bytes_3210(const uint8_t *src, uint8_t *dst, long src_size);
+void shuffle_bytes_012X(const uint8_t *src, uint8_t *dst, long src_size); +void shuffle_bytes_210X(const uint8_t *src, uint8_t *dst, long src_size); +void shuffle_bytes_X012(const uint8_t *src, uint8_t *dst, long src_size); +void shuffle_bytes_X210(const uint8_t *src, uint8_t *dst, long src_size);
the letter F is better than X for 255
+ void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette); void palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette); void palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette); Index: ffmpeg/libswscale/swscale.c =================================================================== --- ffmpeg.orig/libswscale/swscale.c 2010-02-18 23:04:51.000000000 +0100 +++ ffmpeg/libswscale/swscale.c 2010-02-18 23:55:23.000000000 +0100 @@ -1430,6 +1430,11 @@ || (x) == PIX_FMT_ABGR \ )
+#define isRGB24(x) ( \ + (x) == PIX_FMT_RGB24 \ + || (x) == PIX_FMT_BGR24 \ + ) + /* {RGB,BGR}{15,16,24,32,32_1} -> {RGB,BGR}{15,16,24,32} */ static int rgbToRgbWrapper(SwsContext *c, const uint8_t* src[], int srcStride[], int srcSliceY, int srcSliceH, uint8_t* dst[], int dstStride[]) @@ -1458,6 +1463,16 @@ else if (CONV_IS(BGRA, ABGR) || CONV_IS(RGBA, ARGB)) conv = shuffle_bytes_3012; } else + if (isRGB24(srcFormat) && isRGBA32(dstFormat)) { + if (CONV_IS(RGB24, RGBA) + || CONV_IS(BGR24, BGRA)) conv = shuffle_bytes_012X; + else if (CONV_IS(RGB24, ARGB) + || CONV_IS(BGR24, ABGR)) conv = shuffle_bytes_X012; + else if (CONV_IS(RGB24, ABGR) + || CONV_IS(BGR24, ARGB)) conv = shuffle_bytes_X210; + else if (CONV_IS(RGB24, BGRA) + || CONV_IS(BGR24, RGBA)) conv = shuffle_bytes_210X;
this should be using switch/case as a jump table likely is faster also iam against adding a converter for each format pair. there are too many and there are too many that have no practical relevance at all. swscale has a generic converter path, does it even support all these cases? the generic path must be 100% working before a special unscaled converter is added also there must be a speed gain for such a converter and the converter must be usefull in practice. to me this and the related patches feel alot like "adding code because we can add code" not because of anything else IMHO the converters used should be optimized the rest removed. also if the generic path has any problem with rgb this must be fixed not bandaids added to handle a small subset of the cases with huge amounts of code. also 15,16,24,argb,rgba,rgb48le,rgb48be (*2 for rgb/bgr) need almost 200 converters in the style you work here it is completely unacceptable to add that many converteres for unscaled convertion or purely rgb from which not a single one has any practical use as the usefull one have alraedy been implemented years ago. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB it is not once nor twice but times without number that the same ideas make their appearance in the world. -- Aristotle
On date Friday 2010-02-19 01:33:59 +0100, Michael Niedermayer encoded:
On Fri, Feb 19, 2010 at 12:23:36AM +0100, Stefano Sabatini wrote: [...]
Index: ffmpeg/libswscale/swscale.c =================================================================== --- ffmpeg.orig/libswscale/swscale.c 2010-02-18 23:04:51.000000000 +0100 +++ ffmpeg/libswscale/swscale.c 2010-02-18 23:55:23.000000000 +0100 @@ -1430,6 +1430,11 @@ || (x) == PIX_FMT_ABGR \ )
+#define isRGB24(x) ( \ + (x) == PIX_FMT_RGB24 \ + || (x) == PIX_FMT_BGR24 \ + ) + /* {RGB,BGR}{15,16,24,32,32_1} -> {RGB,BGR}{15,16,24,32} */ static int rgbToRgbWrapper(SwsContext *c, const uint8_t* src[], int srcStride[], int srcSliceY, int srcSliceH, uint8_t* dst[], int dstStride[]) @@ -1458,6 +1463,16 @@ else if (CONV_IS(BGRA, ABGR) || CONV_IS(RGBA, ARGB)) conv = shuffle_bytes_3012; } else + if (isRGB24(srcFormat) && isRGBA32(dstFormat)) { + if (CONV_IS(RGB24, RGBA) + || CONV_IS(BGR24, BGRA)) conv = shuffle_bytes_012X; + else if (CONV_IS(RGB24, ARGB) + || CONV_IS(BGR24, ABGR)) conv = shuffle_bytes_X012; + else if (CONV_IS(RGB24, ABGR) + || CONV_IS(BGR24, ARGB)) conv = shuffle_bytes_X210; + else if (CONV_IS(RGB24, BGRA) + || CONV_IS(BGR24, RGBA)) conv = shuffle_bytes_210X;
this should be using switch/case as a jump table likely is faster
also iam against adding a converter for each format pair. there are too many and there are too many that have no practical relevance at all. swscale has a generic converter path, does it even support all these cases?
I currently have no idea, unless you're referring to pixdesc.c:read_line()/write_line(), yes that could work.
the generic path must be 100% working before a special unscaled converter is added also there must be a speed gain for such a converter and the converter must be usefull in practice. to me this and the related patches feel alot like "adding code because we can add code" not because of anything else
Current code is broken in some cases, some bytes are skipped and sometimes there is a buffer overflow.
IMHO the converters used should be optimized the rest removed. also if the generic path has any problem with rgb this must be fixed not bandaids added to handle a small subset of the cases with huge amounts of code.
also 15,16,24,argb,rgba,rgb48le,rgb48be (*2 for rgb/bgr) need almost 200 converters in the style you work here it is completely unacceptable to add that many converteres for unscaled convertion or purely rgb from which not a single one has any practical use as the usefull one have alraedy been implemented years ago.
I agree that is better to have a more generic solution, albeit slower, rather than require special converters for non practically useful scalers. Regards. -- FFmpeg = Fierce and Fantastic Mystic Pitiless Elegant God
On Fri, Feb 19, 2010 at 09:11:11AM +0100, Stefano Sabatini wrote:
On date Friday 2010-02-19 01:33:59 +0100, Michael Niedermayer encoded:
On Fri, Feb 19, 2010 at 12:23:36AM +0100, Stefano Sabatini wrote: [...]
Index: ffmpeg/libswscale/swscale.c =================================================================== --- ffmpeg.orig/libswscale/swscale.c 2010-02-18 23:04:51.000000000 +0100 +++ ffmpeg/libswscale/swscale.c 2010-02-18 23:55:23.000000000 +0100 @@ -1430,6 +1430,11 @@ || (x) == PIX_FMT_ABGR \ )
+#define isRGB24(x) ( \ + (x) == PIX_FMT_RGB24 \ + || (x) == PIX_FMT_BGR24 \ + ) + /* {RGB,BGR}{15,16,24,32,32_1} -> {RGB,BGR}{15,16,24,32} */ static int rgbToRgbWrapper(SwsContext *c, const uint8_t* src[], int srcStride[], int srcSliceY, int srcSliceH, uint8_t* dst[], int dstStride[]) @@ -1458,6 +1463,16 @@ else if (CONV_IS(BGRA, ABGR) || CONV_IS(RGBA, ARGB)) conv = shuffle_bytes_3012; } else + if (isRGB24(srcFormat) && isRGBA32(dstFormat)) { + if (CONV_IS(RGB24, RGBA) + || CONV_IS(BGR24, BGRA)) conv = shuffle_bytes_012X; + else if (CONV_IS(RGB24, ARGB) + || CONV_IS(BGR24, ABGR)) conv = shuffle_bytes_X012; + else if (CONV_IS(RGB24, ABGR) + || CONV_IS(BGR24, ARGB)) conv = shuffle_bytes_X210; + else if (CONV_IS(RGB24, BGRA) + || CONV_IS(BGR24, RGBA)) conv = shuffle_bytes_210X;
this should be using switch/case as a jump table likely is faster
also iam against adding a converter for each format pair. there are too many and there are too many that have no practical relevance at all. swscale has a generic converter path, does it even support all these cases?
I currently have no idea, unless you're referring to pixdesc.c:read_line()/write_line(), yes that could work.
I refered to the scaled converter path not pixdesc.c:read_line()/write_line() [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin
On Thu, Feb 18, 2010 at 9:23 PM, Stefano Sabatini <stefano.sabatini-lala at poste.it> wrote:
On date Thursday 2010-02-18 09:29:19 +0100, Stefano Sabatini encoded:
On date Thursday 2010-02-18 04:10:44 -0200, Ramiro Polla encoded:
On Thu, Feb 18, 2010 at 2:25 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
swscale-test is currently crashing in Windows in ./swscale-test.exe rgb24 argb. Apparently due to heap corruption (that's what windows says), but I couldn't find out the exact cause yet.
It crashes on: ?rgb24 96x96 -> argb ? 96x ?96 flags=16
While trying to free dst in swscale-test.c:doTest() ? ? for (i=0; i<4; i++) { ? ? ? ? av_free(src[i]); ? ? ? ? av_free(dst[i]); ? ? ? ? av_free(out[i]); ? ? }
The function rgb24tobgr32 has overwritten data past the end of its destination pointer because swscale.c:rgbToRgbWrapper() checks: ? ? ? ? if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) ? ? ? ? ? ? dstPtr += ALT32_CORR; and increments dstPtr.
Stefano, I see that you have added that !isRGBA32() there. Could you double check if this is correct for all cases? Shouldn't it actually be:
Index: swscale.c =================================================================== --- swscale.c ? ? ? (revision 30631) +++ swscale.c ? ? ? (working copy) @@ -1502,10 +1502,10 @@ ? ? ?} else { ? ? ? ? ?const uint8_t *srcPtr= src[0]; ? ? ? ? ? ? ? ?uint8_t *dstPtr= dst[0]; - ? ? ? ?if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) + ? ? ? ?if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) ? ? ? ? ? ? ?srcPtr += ALT32_CORR;
- ? ? ? ?if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(srcFormat)) + ? ? ? ?if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat == PIX_FMT_BGR32_1) && !isRGBA32(dstFormat)) ? ? ? ? ? ? ?dstPtr += ALT32_CORR;
? ? ? ? ?if (dstStride[0]*srcBpp == srcStride[0]*dstBpp && srcStride[0] > 0)
No this is wrong.
But I see there is a problem here, we have rgb24to32 writing over the end of the slice. Maybe we need to implement some specific function for dealing with that case (BTW we are also skipping to write the first byte of the ARGB array).
Please try the attached patch.
Things now crash at: rgb565le 96x96 -> abgr 96x 96 flags= 1 SSD= 11, 3, 4, 0
BTW we should find some way to bypass the rgbToRgbWrapper and cache conv function once and for all.
A function pointer to SwsContext should be added. The offset should be done similarly to lumSrcOffset (it might even be reusable).
On Fri, Feb 19, 2010 at 1:31 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
Things now crash at: rgb565le 96x96 -> abgr ? 96x ?96 flags= 1 SSD= ? 11, ? ?3, ? ?4, ? ?0
And now things crash on: rgb24 96x96 -> argb 96x 96 flags= 2 A workaround hack is to allocate more memory for the pictures as in attached patch, but that doesn't fix the issue of writing beyond the slice. I'm sorry to have caused delay on the original patch for lack of testing on Windows. I think it's a good idea to separate the tests so that they may be run in parallel make and also bug reports may be reproduced more quickly (but your patch doesn't take care of parallel make). I now have more time to test the patch on Windows (but I think you can also test MinGW now too). I also think we should get back to thinking on how to implement swscale regression tests in a way that does take many megabytes like the output from swscale-test. It might make more sense to have one ref file for each input file instead of each input/output pair (otherwise we'd need to have 1764 ref files). Ramiro Polla
On Mon, Jul 19, 2010 at 07:47:51PM -0300, Ramiro Polla wrote:
On Fri, Feb 19, 2010 at 1:31 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
Things now crash at: rgb565le 96x96 -> abgr ? 96x ?96 flags= 1 SSD= ? 11, ? ?3, ? ?4, ? ?0
And now things crash on: rgb24 96x96 -> argb 96x 96 flags= 2
A workaround hack is to allocate more memory for the pictures as in attached patch, but that doesn't fix the issue of writing beyond the slice.
I'm sorry to have caused delay on the original patch for lack of testing on Windows. I think it's a good idea to separate the tests so that they may be run in parallel make and also bug reports may be reproduced more quickly (but your patch doesn't take care of parallel make). I now have more time to test the patch on Windows (but I think you can also test MinGW now too).
I also think we should get back to thinking on how to implement swscale regression tests in a way that does take many megabytes like the output from swscale-test. It might make more sense to have one ref file for each input file instead of each input/output pair (otherwise we'd need to have 1764 ref files).
Ramiro Polla
swscale-test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 52eea7c23b081df0a6416a80366622fca31498e0 swscale-test.diff Index: libswscale/swscale-test.c =================================================================== --- libswscale/swscale-test.c (revision 31757) +++ libswscale/swscale-test.c (working copy) @@ -106,9 +106,9 @@ * prefer, as long as they're aligned enough for the architecture, and * they're freed appropriately (such as using av_free for buffers * allocated with av_malloc). */ - src[i]= av_mallocz(srcStride[i]*srcH); - dst[i]= av_mallocz(dstStride[i]*dstH); + src[i]= av_mallocz(srcStride[i]*srcH+16); + dst[i]= av_mallocz(dstStride[i]*dstH+16);
ok
- out[i]= av_mallocz(refStride[i]*h); + out[i]= av_mallocz(refStride[i]*h+16);
not ok [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
On Wed, Jul 21, 2010 at 11:28 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
On Mon, Jul 19, 2010 at 07:47:51PM -0300, Ramiro Polla wrote:
On Fri, Feb 19, 2010 at 1:31 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
Things now crash at: rgb565le 96x96 -> abgr ? 96x ?96 flags= 1 SSD= ? 11, ? ?3, ? ?4, ? ?0
And now things crash on: rgb24 96x96 -> argb ? 96x ?96 flags= 2
A workaround hack is to allocate more memory for the pictures as in attached patch, but that doesn't fix the issue of writing beyond the slice.
I'm sorry to have caused delay on the original patch for lack of testing on Windows. I think it's a good idea to separate the tests so that they may be run in parallel make and also bug reports may be reproduced more quickly (but your patch doesn't take care of parallel make). I now have more time to test the patch on Windows (but I think you can also test MinGW now too).
I also think we should get back to thinking on how to implement swscale regression tests in a way that does take many megabytes like the output from swscale-test. It might make more sense to have one ref file for each input file instead of each input/output pair (otherwise we'd need to have 1764 ref files).
Ramiro Polla
?swscale-test.c | ? ?6 +++--- ?1 file changed, 3 insertions(+), 3 deletions(-) 52eea7c23b081df0a6416a80366622fca31498e0 ?swscale-test.diff Index: libswscale/swscale-test.c =================================================================== --- libswscale/swscale-test.c (revision 31757) +++ libswscale/swscale-test.c (working copy) @@ -106,9 +106,9 @@ ? ? ? ? ? * prefer, as long as they're aligned enough for the architecture, and ? ? ? ? ? * they're freed appropriately (such as using av_free for buffers ? ? ? ? ? * allocated with av_malloc). */ - ? ? ? ?src[i]= av_mallocz(srcStride[i]*srcH); - ? ? ? ?dst[i]= av_mallocz(dstStride[i]*dstH); + ? ? ? ?src[i]= av_mallocz(srcStride[i]*srcH+16); + ? ? ? ?dst[i]= av_mallocz(dstStride[i]*dstH+16);
ok
Applied along with a comment explaining the extra 16 bytes.
Hi Stefano, On Thu, Feb 11, 2010 at 9:32 PM, Stefano Sabatini <stefano.sabatini-lala at poste.it> wrote:
On date Friday 2010-01-29 01:52:23 +0100, Stefano Sabatini encoded:
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
Updated.
What's the speed impact of this change? I suspect it will be much much slower on Windows...
On date Thursday 2010-02-11 22:34:39 -0200, Ramiro Polla encoded:
Hi Stefano,
On Thu, Feb 11, 2010 at 9:32 PM, Stefano Sabatini <stefano.sabatini-lala at poste.it> wrote:
On date Friday 2010-01-29 01:52:23 +0100, Stefano Sabatini encoded:
this is meant to simplify the testing/debuggin work in lsws, and provide a basis for a future lswstest rule, often it happens that I need to test/debug only a particular conversion, I was using the scale lavfi filter for this but it is way easier this way.
Updated.
What's the speed impact of this change? I suspect it will be much much slower on Windows...
stefano at geppetto ~/s/f/libswscale> time --output swscale-test.time swscale-test >/dev/null ^/dev/null stefano at geppetto ~/s/f/libswscale> cat swscale-test.time 250.18user 16.78system 4:27.08elapsed 99%CPU (0avgtext+0avgdata 8976maxresident)k 0inputs+0outputs (0major+10809389minor)pagefaults 0swaps stefano at geppetto ~/s/f/libswscale> quilt push Applying patch implement-swscale-test-lite.patch patching file libswscale/swscale-test-all.sh patching file libswscale/swscale-test.c Now at patch implement-swscale-test-lite.patch stefano at geppetto ~/s/f/libswscale> make swscale-test [...] stefano at geppetto ~/s/f/libswscale> time --output swscale-test-all-sh.time sh swscale-test-all.sh >/dev/null ^/dev/null stefano at geppetto ~/s/f/libswscale> cat swscale-test-all-sh.time 252.80user 24.75system 4:37.75elapsed 99%CPU (0avgtext+0avgdata 8736maxresident)k 0inputs+0outputs (0major+11358266minor)pagefaults 0swaps stefano at geppetto ~/s/f/libswscale> cat /proc/cpuinfo | grep "^model name" model name : Intel(R) Core(TM)2 CPU T7200 @ 2.00GHz model name : Intel(R) Core(TM)2 CPU T7200 @ 2.00GHz There is a trascurable overhead here which is a rather cheap price for the enhanced flexibility and ease for debugging, I don't know how much that will be in windows+MinGW. Regards. -- FFmpeg = Freak & Formidable Minimalistic Purposeless Encoding/decoding Gospel
participants (4)
-
michaelni＠gmx.at -
ramiro.polla＠gmail.com -
stefano.sabatini-lala＠poste.it -
vitor1001＠gmail.com