Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Martin Vignali]

Le lun. 15 oct. 2018 à 11:24, Marc-Antoine ARNAUD <
arnaud.marcanto...@gmail.com> a écrit :

> Hi Martin,
>
> For my point of view, the AVFrame contains the colorspace for each frame,
> which can be different (maybe not volunteer..).
> The colorspace in AVCodecContext is the "pre-computed" colorspace, used to
> generate the output file, as header/footer of wrappers can require
> colorspace informations (like in MXF, MOV, etc.).
>
> So for me, the AVCodecContext is required and needed to be pre-processed to
> wrote the header, and no AVFrame are processed at this stage.
> The AVFrame colorspace is required to maybe change graph properties in
> real-time (like a colorspace conversion can be set to convert the output
> from different colorspace inputs).
>
> So both are differents, but on my point of view, regarding comments, I
> purpose:
> - AVCodecContext will overwrite the output colorspace (if present)
> - AVFrame can be use in video encoder to setup frame headers packets (like
> in Mpeg2, H.264, H.265, ProRes etc.) if no AVCodecContext is configured.
> - a warning needs to be logged if:
> - AVCodecContext colorspace don't match to the AVFrame colorspace
>
> Remark: colorspace mean here variables for colorspace, color primaries and
> color transfer.
>
> So maybe for that, as Paul mentionned, it may require to patch that at an
> upper level than encoder to be sure all works similary. But I don't know
> where to be honest.
>
>
Hello Marc,

From a user point of view :

- if i encode a prores file from a prores file which have different
colorspace in each frame
i expected to have the input colorspace of each frame in the target file.
(So software which use this information will display original and reencode
prores in the same way)
Not sure it's correctly works right now (at least for the prores decoder
part)
In that case colorspace for the muxer, will not be the same than colorspace
for the frame.

- If i would like to set each frame to the right colorspace in a prores
file without reencoding
a bsf (Bitstream filter), can be use to set this info without
decode/reencode
==> Probably not a lot of work, to add this kind of filter

- If i would like to reencode a file, and fix this information at the same
time,
a filter which set the property without converting colorspace can be use
(maybe with a special case to set it only if AVFrame have an unspecifed
colorspace)
(probably need to write one for this)

- If i would like to convert the colorspace of each frame to be sure all
the frames of a file are for example in Rec709
a filter can be use, to convert frame which have another colorspace, and
bypass the frame which already have the right colorspace

For the two last case, i don't know enough libavfilter, to know which is
possible using existing filter,
and what to be add.


For your proposition, i'm not sure auto switching from one way to take
colorspace to another silently
(use AVCodecContext, if AVFrame don't have Colorspace information,  for the
frame header, is a good idea)
Depending of the input file (have information or not for all frames), the
behaviour will be different, probably little hard to use in practice

For the warning if codec context doesn't match AVFrame colorspace,
i agree in theory, but in practice, it can make one warning per frame, and
make ffmpeg log unreadable.


Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Marc-Antoine ARNAUD]

Hi Martin,

For my point of view, the AVFrame contains the colorspace for each frame,
which can be different (maybe not volunteer..).
The colorspace in AVCodecContext is the "pre-computed" colorspace, used to
generate the output file, as header/footer of wrappers can require
colorspace informations (like in MXF, MOV, etc.).

So for me, the AVCodecContext is required and needed to be pre-processed to
wrote the header, and no AVFrame are processed at this stage.
The AVFrame colorspace is required to maybe change graph properties in
real-time (like a colorspace conversion can be set to convert the output
from different colorspace inputs).

So both are differents, but on my point of view, regarding comments, I
purpose:
- AVCodecContext will overwrite the output colorspace (if present)
- AVFrame can be use in video encoder to setup frame headers packets (like
in Mpeg2, H.264, H.265, ProRes etc.) if no AVCodecContext is configured.
- a warning needs to be logged if:
- AVCodecContext colorspace don't match to the AVFrame colorspace

Remark: colorspace mean here variables for colorspace, color primaries and
color transfer.

So maybe for that, as Paul mentionned, it may require to patch that at an
upper level than encoder to be sure all works similary. But I don't know
where to be honest.

Marc-Antoine

Le sam. 13 oct. 2018 à 14:03, Martin Vignali  a
écrit :

> Le mer. 10 oct. 2018 à 17:13, Marc-Antoine ARNAUD <
> arnaud.marcanto...@gmail.com> a écrit :
>
> > I have updated the patch with our discussion.
> > It took information only from the codec context.
> >
> > Marc-Antoine
> >
> > Hello,
>
> If i correctly understand (which is not sure :-) :
>
> the colorspace for AVCodecContext, is when all frame use the same
> colorspace
>
> The colorspace in AVFrame, manage the case where colorspace can not be the
> same inside each frame.
> and this info is use inside filtering graph.
>
> If this is correct, then prores encoder need to use AVFrame colorspace for
> each frame
> and if the colorspace information is wrong, or if user need to set another
> one, a filter need to be use to edit the frame (set colorspace information
> or convert from one colorspace to another frame by frame)
> (Or if just the colorspace metadata inside the compress prores frame need
> to be fix,  a bsf can be write to edit frame colorspace properties for
> prores frame)
>
> Another way can be to add an option to the prores encoder, to take
> colorspace information from CodecContext or from Frame.
>
>
> But if other people think your way is better,
> i will change my patch for prores_aw, in order to use the same way to set
> colorspace for each prores frame.
>
> P.S. : Seems like png at least also use AVFrame information instead of
> AVCodecContext
>
> Martin
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Martin Vignali]

Le mer. 10 oct. 2018 à 17:13, Marc-Antoine ARNAUD <
arnaud.marcanto...@gmail.com> a écrit :

> I have updated the patch with our discussion.
> It took information only from the codec context.
>
> Marc-Antoine
>
> Hello,

If i correctly understand (which is not sure :-) :

the colorspace for AVCodecContext, is when all frame use the same colorspace

The colorspace in AVFrame, manage the case where colorspace can not be the
same inside each frame.
and this info is use inside filtering graph.

If this is correct, then prores encoder need to use AVFrame colorspace for
each frame
and if the colorspace information is wrong, or if user need to set another
one, a filter need to be use to edit the frame (set colorspace information
or convert from one colorspace to another frame by frame)
(Or if just the colorspace metadata inside the compress prores frame need
to be fix,  a bsf can be write to edit frame colorspace properties for
prores frame)

Another way can be to add an option to the prores encoder, to take
colorspace information from CodecContext or from Frame.


But if other people think your way is better,
i will change my patch for prores_aw, in order to use the same way to set
colorspace for each prores frame.

P.S. : Seems like png at least also use AVFrame information instead of
AVCodecContext

Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Marc-Antoine ARNAUD]

I have updated the patch with our discussion.
It took information only from the codec context.

Marc-Antoine

Le mer. 10 oct. 2018 à 11:36, Paul B Mahol  a écrit :

> On 10/10/18, Marc-Antoine ARNAUD  wrote:
> > For me it's the only codec who use picture colorspace as source.
> > All others uses only the CodecContext.
> > I don't know the exact reason, but I suppose it can be easiest to manage
> > output colorspace during merge of video, as a video can have only one
> > "static" video colorspace.
> >
> > So for me it made sense to keep that patch. Maybe with removing the
> `else`
> > to don't take colorspace from pictures as other codecs can do.
>
> That seems reasonable. What other people think?
>
> >
> > Marc-Antoine
> >
> > Le ven. 5 oct. 2018 `a 10:15, Paul B Mahol  a ecrit :
> >
> >> On 10/5/18, Marc-Antoine ARNAUD  wrote:
> >> > In our case we have some files with bad colorspaces (in HD but with
> >> > bt601
> >> > colorspace).
> >> > So we use -colorspace, -color_trc, -color_primaries to force the
> output
> >> > colorspace.
> >> >
> >> > We keep compatibility with "old command line", we get source
> colorspace
> >> if
> >> > nothing is mentionned.
> >> > It work like that for Mpeg2video codec, so we expect to have the same
> >> here.
> >>
> >> Correct patch should be one that changes frame properties, otherwise
> >> every encoder that uses these properties needs to be updated with extra
> >> lines to maintain.
> >>
> >> >
> >> > Marc-Antoine
> >> >
> >> >
> >> > Le jeu. 4 oct. 2018 `a 18:36, Paul B Mahol  a
> ecrit :
> >> >
> >> >> On 10/4/18, Marc-Antoine ARNAUD 
> wrote:
> >> >> >
> >> >> >
> >> >>
> >> >> Why?
> >> >>
> >> >> IIRC this patch is not needed.
> >> >> ___
> >> >> ffmpeg-devel mailing list
> >> >> ffmpeg-devel@ffmpeg.org
> >> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >>
> >> > ___
> >> > ffmpeg-devel mailing list
> >> > ffmpeg-devel@ffmpeg.org
> >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >> ___
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >
> >
> > --
> > *Marc-Antoine*
> > |e:arnaud.marcanto...@gmail.com
> > |tel: 06-84-71-84-45 <06%2084%2071%2084%2045>
> > | ohloh: http://bit.ly/1iwtlsU
> > [image: LinkedIn]
> > <
> http://s.wisestamp.com/links?url=https%3A%2F%2Fwww.linkedin.com%2Fpub%2Fmarc-antoine-arnaud%2Fb%2F7b8%2F2a3=YXJuYXVkLm1hcmNhbnRvaW5lQGdtYWlsLmNvbQ%3D%3D
> >
> > [image:
> > Google Plus]
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


0001-prores_ks-use-CodecContext-for-color-information.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Paul B Mahol]

On 10/10/18, Marc-Antoine ARNAUD  wrote:
> For me it's the only codec who use picture colorspace as source.
> All others uses only the CodecContext.
> I don't know the exact reason, but I suppose it can be easiest to manage
> output colorspace during merge of video, as a video can have only one
> "static" video colorspace.
>
> So for me it made sense to keep that patch. Maybe with removing the `else`
> to don't take colorspace from pictures as other codecs can do.

That seems reasonable. What other people think?

>
> Marc-Antoine
>
> Le ven. 5 oct. 2018 `a 10:15, Paul B Mahol  a ecrit :
>
>> On 10/5/18, Marc-Antoine ARNAUD  wrote:
>> > In our case we have some files with bad colorspaces (in HD but with
>> > bt601
>> > colorspace).
>> > So we use -colorspace, -color_trc, -color_primaries to force the output
>> > colorspace.
>> >
>> > We keep compatibility with "old command line", we get source colorspace
>> if
>> > nothing is mentionned.
>> > It work like that for Mpeg2video codec, so we expect to have the same
>> here.
>>
>> Correct patch should be one that changes frame properties, otherwise
>> every encoder that uses these properties needs to be updated with extra
>> lines to maintain.
>>
>> >
>> > Marc-Antoine
>> >
>> >
>> > Le jeu. 4 oct. 2018 `a 18:36, Paul B Mahol  a ecrit :
>> >
>> >> On 10/4/18, Marc-Antoine ARNAUD  wrote:
>> >> >
>> >> >
>> >>
>> >> Why?
>> >>
>> >> IIRC this patch is not needed.
>> >> ___
>> >> ffmpeg-devel mailing list
>> >> ffmpeg-devel@ffmpeg.org
>> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >>
>> > ___
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
>
> --
> *Marc-Antoine*
> |e:arnaud.marcanto...@gmail.com
> |tel: 06-84-71-84-45
> | ohloh: http://bit.ly/1iwtlsU
> [image: LinkedIn]
> 
> [image:
> Google Plus]
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Marc-Antoine ARNAUD]

For me it's the only codec who use picture colorspace as source.
All others uses only the CodecContext.
I don't know the exact reason, but I suppose it can be easiest to manage
output colorspace during merge of video, as a video can have only one
"static" video colorspace.

So for me it made sense to keep that patch. Maybe with removing the `else`
to don't take colorspace from pictures as other codecs can do.

Marc-Antoine

Le ven. 5 oct. 2018 à 10:15, Paul B Mahol  a écrit :

> On 10/5/18, Marc-Antoine ARNAUD  wrote:
> > In our case we have some files with bad colorspaces (in HD but with bt601
> > colorspace).
> > So we use -colorspace, -color_trc, -color_primaries to force the output
> > colorspace.
> >
> > We keep compatibility with "old command line", we get source colorspace
> if
> > nothing is mentionned.
> > It work like that for Mpeg2video codec, so we expect to have the same
> here.
>
> Correct patch should be one that changes frame properties, otherwise
> every encoder that uses these properties needs to be updated with extra
> lines to maintain.
>
> >
> > Marc-Antoine
> >
> >
> > Le jeu. 4 oct. 2018 `a 18:36, Paul B Mahol  a ecrit :
> >
> >> On 10/4/18, Marc-Antoine ARNAUD  wrote:
> >> >
> >> >
> >>
> >> Why?
> >>
> >> IIRC this patch is not needed.
> >> ___
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


-- 
*Marc-Antoine*
|e:arnaud.marcanto...@gmail.com
|tel: 06-84-71-84-45
| ohloh: http://bit.ly/1iwtlsU
[image: LinkedIn]

[image:
Google Plus]
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Paul B Mahol]

On 10/5/18, Marc-Antoine ARNAUD  wrote:
> In our case we have some files with bad colorspaces (in HD but with bt601
> colorspace).
> So we use -colorspace, -color_trc, -color_primaries to force the output
> colorspace.
>
> We keep compatibility with "old command line", we get source colorspace if
> nothing is mentionned.
> It work like that for Mpeg2video codec, so we expect to have the same here.

Correct patch should be one that changes frame properties, otherwise
every encoder that uses these properties needs to be updated with extra
lines to maintain.

>
> Marc-Antoine
>
>
> Le jeu. 4 oct. 2018 `a 18:36, Paul B Mahol  a ecrit :
>
>> On 10/4/18, Marc-Antoine ARNAUD  wrote:
>> >
>> >
>>
>> Why?
>>
>> IIRC this patch is not needed.
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Marc-Antoine ARNAUD]

In our case we have some files with bad colorspaces (in HD but with bt601
colorspace).
So we use -colorspace, -color_trc, -color_primaries to force the output
colorspace.

We keep compatibility with "old command line", we get source colorspace if
nothing is mentionned.
It work like that for Mpeg2video codec, so we expect to have the same here.

Marc-Antoine


Le jeu. 4 oct. 2018 à 18:36, Paul B Mahol  a écrit :

> On 10/4/18, Marc-Antoine ARNAUD  wrote:
> >
> >
>
> Why?
>
> IIRC this patch is not needed.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] prores_ks: use CodecContext for color information if specified

[Paul B Mahol]

On 10/4/18, Marc-Antoine ARNAUD  wrote:
>
>

Why?

IIRC this patch is not needed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel