On Wed, Aug 02, 2017 at 03:32:04PM +0100, James Cowgill wrote: > Hi, > > On 02/08/17 14:18, Michael Niedermayer wrote: > > On Tue, Aug 01, 2017 at 02:46:22PM +0100, James Cowgill wrote: > >> Fixes filter-pixfmts-scale test failing on big-endian systems due to > >> alpSrc not being cast to (const int32_t**). > >> > >> Also fixes distortions in the output alpha channel values by copying the > >> alpha channel code from the rgba64 case found elsewhere in output.c. > >> > >> Fixes ticket 6555. > >> > >> Signed-off-by: James Cowgill <james.cowg...@imgtec.com> > >> --- > >> libswscale/output.c | 15 ++++++++------- > >> tests/ref/fate/filter-pixfmts-scale | 4 ++-- > >> 2 files changed, 10 insertions(+), 9 deletions(-) > >> > >> diff --git a/libswscale/output.c b/libswscale/output.c > >> index 9774e9f327..8e5ec0a256 100644 > >> --- a/libswscale/output.c > >> +++ b/libswscale/output.c > >> @@ -2026,17 +2026,18 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t > >> *lumFilter, > >> const int16_t **lumSrcx, int lumFilterSize, > >> const int16_t *chrFilter, const int16_t **chrUSrcx, > >> const int16_t **chrVSrcx, int chrFilterSize, > >> - const int16_t **alpSrc, uint8_t **dest, > >> + const int16_t **alpSrcx, uint8_t **dest, > >> int dstW, int y) > >> { > >> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(c->dstFormat); > >> int i; > >> - int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrc; > >> + int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrcx; > >> uint16_t **dest16 = (uint16_t**)dest; > >> const int32_t **lumSrc = (const int32_t**)lumSrcx; > >> const int32_t **chrUSrc = (const int32_t**)chrUSrcx; > >> const int32_t **chrVSrc = (const int32_t**)chrVSrcx; > >> - int A = 0; // init to silence warning > >> + const int32_t **alpSrc = (const int32_t**)alpSrcx; > > > >> + int A = 0xFFFF << 14; > > > > unused value > > The initial value of A is unused in the old code, but not in the new code.
IIRC all uses are under hasAlpha and it is writen to in that case first > > >> > >> for (i = 0; i < dstW; i++) { > >> int j; > >> @@ -2059,13 +2060,13 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t > >> *lumFilter, > >> V >>= 14; > >> > >> if (hasAlpha) { > >> - A = 1 << 18; > >> + A = -0x40000000; > > > > where does this value come from ? > > it looks copy and pasted from luma, but alpha does not have a black > > level offset as its not luminance > > I confess I only know the basics of how these functions work. On the > basis that yuv2gbrp_full_X_c looks like it copies yuv2rgb_X_c_template, > and I would have thought the rgb and gbr cases should be similar, I > copied a number of things from yuv2rgba64_full_X_c_template into this > function. That value and all of the modifications inside the for loop > come from there. [...] > This is where the clipping code in yuv2rgba64_full_X_c_template is, and > in that function, the value of A is not clipped - only the value stored > in dest. hmm, ok, on a 2nd look this is probably correct [...] -- 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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel