[vlc-devel] [patch] esformat rgb improvements

jnqnfe at gmail.com jnqnfe at gmail.com
Thu May 10 06:49:01 CEST 2018


patch set
=========

#1: esformat: remove old fixme

redundant as of 1239429ee6aed130a2e35e679643fdff135c24b3

it was copied to esformat in the commit prior to this (including
addition of the fixme), then the original in video_output was removed
in favour of using the esformat's video_format_FixRgb function, but the
fixme was left behind.

#2: esformat: group rgb masks and shifts in sub-structs

Also add (more) documentation to explain things to those for whom the
purpose and use of the shifts is not necessarily so obvious.

Bonus: simplifies copying in filter_NewBlend

#3: esformat: improve MaskToShift docs
#4: esformat: improve video_format_FixRgb docs

#5: esformat: improve clarity of MaskToShift

no change in usage or output (for good masks, see #7!)

#6: esformat: add assert to MaskToShift

this actually catches an issue with the svg codec, see below.

#7: esformat: temporary svg fix

Only masks of continuous blocks of 1's are valid to give to
MaskToShift() (via video_format_FixRgb()), however the svg codec module
uses a red mask of 0x80800000.
    
This previously was not a problem, the code spat out rrshift=0 and
lrshift=23, though it is not clear whether those produce correct/useful
results. With the newly tweaked code this "bad" mask causes a problem.
The same lrshift is returned, but rrshift=-1 and now trips up the
assert. This hack fixes the situation temporarily, until a better
solution is found.

Further discussing the svg red mask situation
=============================================

the svg codec module uses a value of 0x80800000 as a red mask. This
comes from f68056f174e904b280f05d133e25481a24c99383. MaskToShift was
surely written to expect only masks that are continous blocks of 1's -
walk through its implementation! This mask is thus not garuanteed to
work, and the tweaks made in the attached patchset now highlight the
problem.

I am not at all clear why this mask has the value it does, not having
done any research into the librsvg (yet), it seems very odd. Perhaps
the original author could comment?

Ideally this mask would get fixed to some valid value and patch #7
could be dropped.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 01.patch
Type: text/x-patch
Size: 567 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180510/7c287f41/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 02.patch
Type: text/x-patch
Size: 48628 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180510/7c287f41/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 03.patch
Type: text/x-patch
Size: 765 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180510/7c287f41/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 04.patch
Type: text/x-patch
Size: 857 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180510/7c287f41/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 05.patch
Type: text/x-patch
Size: 2691 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180510/7c287f41/attachment-0011.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 06.patch
Type: text/x-patch
Size: 699 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180510/7c287f41/attachment-0012.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 07.patch
Type: text/x-patch
Size: 1293 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180510/7c287f41/attachment-0013.bin>


More information about the vlc-devel mailing list