Re: [FFmpeg-devel] [PATCH 1/3] avfilter: Add support for colour range as a link parameter

[Paul B Mahol]

On 7/25/18, Kieran O Leary  wrote:
> Hi,
>
> On Thu, Feb 22, 2018 at 4:15 PM, Philip Langdale  wrote:
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> On Thu, 22 Feb 2018 12:39:16 +0100
>> Nicolas George  wrote:
>>
>>> Philip Langdale (2018-02-21):
>>> > Negotiation is part of Paul's larger changeset, and will be a useful
>>> > feature. My change is still a strict improvement over the current
>>> > state of the world - where range is not propagated at all,
>>> > regardless of compatibility. In those situations where negotiation
>>> > is required, the status quo will essentially continue, with the
>>> > range value not accurately reflecting the frame contents.
>>>
>>> I am not comfortable with what you write here.
>>>
>>> I am afraid that adding negotiation on top of this would be more work
>>> than adding negotiation on top of the current code.
>>>
>>> I am also afraid that an incorrect value is worse than an unspecified
>>> one.
>>>
>>> But it all depends on what filters and codecs actually do with the
>>> color range, and that I do not know.
>>>
>>> Could you perhaps make a little summary of that issue: where the color
>>> range comes from, which filters and encoders do not care, which ones
>>> only work with one, which ones do something special with it? Maybe as
>>> a longer doxy comment for enum AVColorRange in libavutil/pixfmt.h?
>>
>> You can go back and read through Paul's patchset from december, which
>> implements negotiation. My changes here are a strict subset of those,
>> so empirically, merging this subset on its own does not make
>> negotiation harder. You are also welcome to recommend merging his full
>> patchset now; it never got the reviews it needed, but that doesn't mean
>> it can't.
>>
>
> I am hoping to test out this patchset in the hopes that it could
> rectify the issue we're having with colour metadata in a MOV/ProRes
> file not being migrated over to Matroska:
> https://ffmpeg.org/pipermail/ffmpeg-user/2018-July/040717.html
>
> I have two questions as I'm a bit confused:
> 1. Does this patchset require Paul's patchset to be merged? I can't
> tell if it has already or not.
> 2. I see that this thread describes a series of 4 patches, but there
> also seems to be
> a set of three patches from February - do the set of 3 supercede this
> set of 4? https://patchwork.ffmpeg.org/patch/7694/
>

IIRC this patchset have been applied, you have color range in lavfi filtergraph.
But it is not used for negotiation

Also this patchset only plays with color range and other properties
are not touched.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avfilter: Add support for colour range as a link parameter

[Kieran O Leary]

Hi,

On Thu, Feb 22, 2018 at 4:15 PM, Philip Langdale  wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On Thu, 22 Feb 2018 12:39:16 +0100
> Nicolas George  wrote:
>
>> Philip Langdale (2018-02-21):
>> > Negotiation is part of Paul's larger changeset, and will be a useful
>> > feature. My change is still a strict improvement over the current
>> > state of the world - where range is not propagated at all,
>> > regardless of compatibility. In those situations where negotiation
>> > is required, the status quo will essentially continue, with the
>> > range value not accurately reflecting the frame contents.
>>
>> I am not comfortable with what you write here.
>>
>> I am afraid that adding negotiation on top of this would be more work
>> than adding negotiation on top of the current code.
>>
>> I am also afraid that an incorrect value is worse than an unspecified
>> one.
>>
>> But it all depends on what filters and codecs actually do with the
>> color range, and that I do not know.
>>
>> Could you perhaps make a little summary of that issue: where the color
>> range comes from, which filters and encoders do not care, which ones
>> only work with one, which ones do something special with it? Maybe as
>> a longer doxy comment for enum AVColorRange in libavutil/pixfmt.h?
>
> You can go back and read through Paul's patchset from december, which
> implements negotiation. My changes here are a strict subset of those,
> so empirically, merging this subset on its own does not make
> negotiation harder. You are also welcome to recommend merging his full
> patchset now; it never got the reviews it needed, but that doesn't mean
> it can't.
>

I am hoping to test out this patchset in the hopes that it could
rectify the issue we're having with colour metadata in a MOV/ProRes
file not being migrated over to Matroska:
https://ffmpeg.org/pipermail/ffmpeg-user/2018-July/040717.html

I have two questions as I'm a bit confused:
1. Does this patchset require Paul's patchset to be merged? I can't
tell if it has already or not.
2. I see that this thread describes a series of 4 patches, but there
also seems to be
a set of three patches from February - do the set of 3 supercede this
set of 4? https://patchwork.ffmpeg.org/patch/7694/

Best,

Kieran O'Leary
IFI Irish Film Archive
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avfilter: Add support for colour range as a link parameter

[Philip Langdale]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Thu, 22 Feb 2018 12:39:16 +0100
Nicolas George  wrote:

> Philip Langdale (2018-02-21):
> > Negotiation is part of Paul's larger changeset, and will be a useful
> > feature. My change is still a strict improvement over the current
> > state of the world - where range is not propagated at all,
> > regardless of compatibility. In those situations where negotiation
> > is required, the status quo will essentially continue, with the
> > range value not accurately reflecting the frame contents.  
> 
> I am not comfortable with what you write here.
> 
> I am afraid that adding negotiation on top of this would be more work
> than adding negotiation on top of the current code.
> 
> I am also afraid that an incorrect value is worse than an unspecified
> one.
> 
> But it all depends on what filters and codecs actually do with the
> color range, and that I do not know.
> 
> Could you perhaps make a little summary of that issue: where the color
> range comes from, which filters and encoders do not care, which ones
> only work with one, which ones do something special with it? Maybe as
> a longer doxy comment for enum AVColorRange in libavutil/pixfmt.h?

You can go back and read through Paul's patchset from december, which
implements negotiation. My changes here are a strict subset of those,
so empirically, merging this subset on its own does not make
negotiation harder. You are also welcome to recommend merging his full
patchset now; it never got the reviews it needed, but that doesn't mean
it can't.

Thanks,

- --phil
-BEGIN PGP SIGNATURE-

iEYEARECAAYFAlqO7JYACgkQ+NaxlGp1aC7HKQCgzVxU3TSTxQeZgNA5YdZOEzaE
Go0An1Jla+EPtaWOENJRcETeJh2SG3l6
=lX4D
-END PGP SIGNATURE-
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avfilter: Add support for colour range as a link parameter

[Nicolas George]

Philip Langdale (2018-02-21):
> Negotiation is part of Paul's larger changeset, and will be a useful
> feature. My change is still a strict improvement over the current state
> of the world - where range is not propagated at all, regardless of
> compatibility. In those situations where negotiation is required, the
> status quo will essentially continue, with the range value not
> accurately reflecting the frame contents.

I am not comfortable with what you write here.

I am afraid that adding negotiation on top of this would be more work
than adding negotiation on top of the current code.

I am also afraid that an incorrect value is worse than an unspecified
one.

But it all depends on what filters and codecs actually do with the color
range, and that I do not know.

Could you perhaps make a little summary of that issue: where the color
range comes from, which filters and encoders do not care, which ones
only work with one, which ones do something special with it? Maybe as a
longer doxy comment for enum AVColorRange in libavutil/pixfmt.h?


(Also, my mail contained "reply-to: ffmpeg-devel"; please heed it in the
future.)

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avfilter: Add support for colour range as a link parameter

[Philip Langdale]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Wed, 21 Feb 2018 23:37:57 +0100
Nicolas George  wrote:

> Philip Langdale (2018-02-21):
> > As part of achieving our YUVJ-free future, it needs to be possible
> > to pass the colour range property from a decoder context to an
> > encoder context. In the absence of filters, this is obviously
> > trivial, but as soon as filters are introduced, there needs to
> > be a way to pass and preserve the property (or modify it, as
> > some filters will do).
> > 
> > Based on existing properties of this type, this change adds a
> > link property and ways to set it from a buffer source and get it
> > from a buffer sink.  
> 
> I am not sure when I will be able to give a close look at the code,
> so I make my general comment immediately:
> 
> Are you sure this is a property that must be forwarded and not
> negotiated?
> 
> You seem to say that encoders may support a limited set of ranges. It
> looks like the range should be constrained by the output, like the
> pixel format, and negotiated on the chain to insert a conversion
> filter if necessary.
> 
> But I am not familiar with color ranges. I just want to make sure you
> have considered the question.
> 
> Regards,
> 

Negotiation is part of Paul's larger changeset, and will be a useful
feature. My change is still a strict improvement over the current state
of the world - where range is not propagated at all, regardless of
compatibility. In those situations where negotiation is required, the
status quo will essentially continue, with the range value not
accurately reflecting the frame contents.

- --phil
-BEGIN PGP SIGNATURE-

iEYEARECAAYFAlqN9woACgkQ+NaxlGp1aC6DvgCfaDsw2OsmY0xkXgajn6t7vWKx
/XsAoIl2B760kvv8PPRxWdkGUbV/tCdv
=F60C
-END PGP SIGNATURE-
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] avfilter: Add support for colour range as a link parameter

[Nicolas George]

Philip Langdale (2018-02-21):
> As part of achieving our YUVJ-free future, it needs to be possible
> to pass the colour range property from a decoder context to an
> encoder context. In the absence of filters, this is obviously
> trivial, but as soon as filters are introduced, there needs to
> be a way to pass and preserve the property (or modify it, as
> some filters will do).
> 
> Based on existing properties of this type, this change adds a
> link property and ways to set it from a buffer source and get it
> from a buffer sink.

I am not sure when I will be able to give a close look at the code, so I
make my general comment immediately:

Are you sure this is a property that must be forwarded and not
negotiated?

You seem to say that encoders may support a limited set of ranges. It
looks like the range should be constrained by the output, like the pixel
format, and negotiated on the chain to insert a conversion filter if
necessary.

But I am not familiar with color ranges. I just want to make sure you
have considered the question.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel