[FFmpeg-devel] [PATCH] avfilter: add unpremultiply filter

[Tobias Rapp]

On 01.08.2017 17:33, Paul B Mahol wrote:
> On 8/1/17, Tobias Rapp <t.rapp at noa-archive.com> wrote:
>> On 01.08.2017 15:31, Paul B Mahol wrote:
>>> On 8/1/17, Tobias Rapp <t.rapp at noa-archive.com> wrote:
>>>> On 01.08.2017 13:03, Paul B Mahol wrote:
>>>>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>>>>> ---
>>>>>  doc/filters.texi             |  13 ++
>>>>>  libavfilter/Makefile         |   1 +
>>>>>  libavfilter/allfilters.c     |   1 +
>>>>>  libavfilter/vf_premultiply.c | 307
>>>>> ++++++++++++++++++++++++++++++++++++-------
>>>>>  4 files changed, 277 insertions(+), 45 deletions(-)
>>>>>
>>>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>>>> index 4089135..a50696a 100644
>>>>> --- a/doc/filters.texi
>>>>> +++ b/doc/filters.texi
>>>>> @@ -14532,6 +14532,19 @@ ffmpeg -i INPUT -vf trim=duration=1
>>>>>
>>>>>  @end itemize
>>>>>
>>>>> + at section unpremultiply
>>>>> +Apply alpha unpremultiply effect to input video stream using first
>>>>> plane
>>>>> +of second stream as alpha.
>>>>> +
>>>>> +Both streams must have same dimensions and same pixel format.
>>>>> +
>>>>> +The filter accepts the following option:
>>>>> +
>>>>> + at table @option
>>>>> + at item planes
>>>>> +Set which planes will be processed, unprocessed planes will be copied.
>>>>> +By default value 0xf, all planes will be processed.
>>>>> + at end table
>>>>
>>>> IMHO using a flags-like string "planes=rgb" would be more user-friendly
>>>> than a bitmask. At least the documentation should tell which bit refers
>>>> to what channel.
>>>
>>> It is directly related to pixel format.
>>
>> I'm just wondering whether I can apply the logic of
>> AVPixFmtDescriptor.comp to the planes bitmask or not:
>>
>>      /**
>>       * Parameters that describe how pixels are packed.
>>       * If the format has 1 or 2 components, then luma is 0.
>>       * If the format has 3 or 4 components:
>>       *   if the RGB flag is set then 0 is red, 1 is green and 2 is blue;
>>       *   otherwise 0 is luma, 1 is chroma-U and 2 is chroma-V.
>>       *
>>       * If present, the Alpha channel is always the last component.
>>       */
>
> I mainly did it bitmask way because of additional code needed to handle cases
> when there is no r/g/b but y/u/v planes and user supplied r/g/b only.


Indeed a bitmask is more generic. I'm not against it but think that 
there should be some more details in the user documentation on how to 
map the bits to planes. For example (in case the code comment above 
applies) something like:

"""
If the format has 1 or 2 components, then luma is bit 0.
If the format has 3 or 4 components:
   for RGB formats bit 0 is red, bit 1 is green and bit 2 is blue;
   for YUV formats bit 0 is luma, bit 1 is chroma-U and bit 2 is chroma-V.
If present, the alpha channel is always the last bit.
"""

Regards,
Tobias