On Tue, Jan 28, 2020 at 22:42:33 -0600, rcombs wrote:
> > You're inverting the "leftmost" 8 bits of c? Can't you just make this
> > return (c ^ 0xff000000);
> > ? Even inline, or just a macro? (Or perhaps I'm missing something, and
> > (255 - a) is not always the same as (a ^ 0xff).
>
> These are indeed equivalent, but I thought this more clearly conveyed
> the intent here ("convert between 0=transparent and 0=opaque by
> inverting the 0-255 alpha field").
Whatever is more clear to you. ;) I though it overcomplicated the whole
process, but perhaps it doesn't matter (I don't decide whether it
does).
> > To make this more readable, you should also MACROify the "offsetof(cls,
> > obj.X)", as many other options sections do it, and perhaps align the
> > entries' columns.
>
> I shied away from this to avoid polluting the global macro space
> (since this is in a header); I can do it anyway if you prefer (it's
> not like it's a public header, at least).
As you write, it's not global space. Anyway, I recall seeing such
macros (for multiple use of options structs) in the actual
implementation (*.c), not the declaration. If that reduces your clutter
concern.
Cheers,
looking forward to other proper reviews of your contributions ;-)
Moritz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
