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 <[email protected]> > >> --- > >> 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 [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
