Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Derek Buitenhuis]

On 11/22/2017 1:33 PM, Kevin Wheatley wrote:
> The guess work comes from this list:
> 
> https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-SAMPLE__COLR__SETTINGS

It's a mistake to conflate resolutions with the standards listed there. It does 
not
list any heuristics, just a set of standards and associated values. No info 
falls
under "Anything Else", IMO.

> and I agree is the source of a lot of chaos, but it is what several
> tools have done in the absence of any specified values.

"Other tools use heuristics too" is a bad rationale for creating such files by 
default.

> My usage of the feature is to write the atom when I need to by
> explicitly enabling it on the command line and I also explicitly set
> the metadata either from the input file, or by overriding on the
> command line.

This use-case is preserved if we leave the heuristics as an option, which I'm
totally OK with.

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Kevin Wheatley]

The guess work comes from this list:

https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-SAMPLE__COLR__SETTINGS

and I agree is the source of a lot of chaos, but it is what several
tools have done in the absence of any specified values.

My usage of the feature is to write the atom when I need to by
explicitly enabling it on the command line and I also explicitly set
the metadata either from the input file, or by overriding on the
command line.

Kevin


On Mon, Nov 20, 2017 at 4:52 PM, Derek Buitenhuis
 wrote:
> On 11/20/2017 4:45 PM, Dave Rice wrote:
>> What do you propose as the default for guessed_hacky_crap? Also are there 
>> supporters for the need of a guessed_hacky_crap optio? Is there precedence 
>> in ffmpeg to enable/disable guesswork via a user option?
>
> I personally would never use the guesswork stuff, since I think it is a
> terrible idea, so can't really propose a default or support it.
>
> The guesswork stuff was added at the same time as colr support, by Michael,
> in 7b6f4191763a5ffde02fa198101aabc3ddb5bf61, so maybe he has some opinion.
>
> - Derek
> ___
> 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] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Derek Buitenhuis]

On 11/20/2017 4:45 PM, Dave Rice wrote:
> What do you propose as the default for guessed_hacky_crap? Also are there 
> supporters for the need of a guessed_hacky_crap optio? Is there precedence in 
> ffmpeg to enable/disable guesswork via a user option?

I personally would never use the guesswork stuff, since I think it is a
terrible idea, so can't really propose a default or support it.

The guesswork stuff was added at the same time as colr support, by Michael,
in 7b6f4191763a5ffde02fa198101aabc3ddb5bf61, so maybe he has some opinion.

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Dave Rice]

> On Nov 20, 2017, at 11:22 AM, Derek Buitenhuis  
> wrote:
> 
> On 11/20/2017 3:19 PM, Dave Rice wrote:
>> TN2162 requires a colr atom for uncompressed yuv (including v210, v308, 
>> v408, etc) in mov, so I'd prefer to write it in this case. Note that the 
>> colr atom provides an option for unspecified for each of the color values, 
>> so there's a method to write a colr atom which basically says ¯\_(ツ)_/¯.
> 
> [...]
> 
>> I disagree. I'd prefer to follow the specification and write a colr atom (in 
>> the case of uncompressed yuv in mov) that say the colr is unspecified rather 
>> than to write no colr atom at all and create an invalid file. See 
>> https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG9.
>> 
>> I do agree that guesswork should be avoided or provided under an option that 
>> users could opt into. I suppose my preference order from support to oppose 
>> for writing uncompressed yuv in mov would be:
>> 
>> 1: write a colr atom in all cases (if unknown, use unspecified values in 
>> colr atom)
>> 2: write a colr atom if color data the known (no colr atom if unknown)
>> 3: write a colr atom in all cases (if unknown, just make stuff up, #yolo)
> 
> My opinion falls somewhere in between, to something like this (in 
> pseudo-code):
> 
>if (colors_known) {
>write_colr(vals);
>} else if (uncompressed_yuv) {
>if (guesswork_user_option) {
>write_colr(guessed_hacky_crap);
>} else {
>write_colr(unspecified);
>}
>} else if (guesswork_user_option) {
>write_colr(guessed_hacky_crap);
>} else {
>// Don't write a colr atom because it adds no value, to write 
> unspecified in it
>// and no spec requires it for compressed streams.
>}

What do you propose as the default for guessed_hacky_crap? Also are there 
supporters for the need of a guessed_hacky_crap optio? Is there precedence in 
ffmpeg to enable/disable guesswork via a user option?
Dave
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Derek Buitenhuis]

On 11/20/2017 3:19 PM, Dave Rice wrote:
> TN2162 requires a colr atom for uncompressed yuv (including v210, v308, v408, 
> etc) in mov, so I'd prefer to write it in this case. Note that the colr atom 
> provides an option for unspecified for each of the color values, so there's a 
> method to write a colr atom which basically says ¯\_(ツ)_/¯.

[...]

> I disagree. I'd prefer to follow the specification and write a colr atom (in 
> the case of uncompressed yuv in mov) that say the colr is unspecified rather 
> than to write no colr atom at all and create an invalid file. See 
> https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG9.
> 
> I do agree that guesswork should be avoided or provided under an option that 
> users could opt into. I suppose my preference order from support to oppose 
> for writing uncompressed yuv in mov would be:
> 
> 1: write a colr atom in all cases (if unknown, use unspecified values in colr 
> atom)
> 2: write a colr atom if color data the known (no colr atom if unknown)
> 3: write a colr atom in all cases (if unknown, just make stuff up, #yolo)

My opinion falls somewhere in between, to something like this (in pseudo-code):

if (colors_known) {
write_colr(vals);
} else if (uncompressed_yuv) {
if (guesswork_user_option) {
write_colr(guessed_hacky_crap);
} else {
write_colr(unspecified);
}
} else if (guesswork_user_option) {
write_colr(guessed_hacky_crap);
} else {
// Don't write a colr atom because it adds no value, to write 
unspecified in it
// and no spec requires it for compressed streams.
}

Does this make sense?

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Dave Rice]

> On Nov 20, 2017, at 9:55 AM, Derek Buitenhuis  
> wrote:
> 
> On 11/20/2017 2:50 PM, Dave Rice wrote:
>> I am hesitant to see write_colr option removed since the guesswork used here 
>> https://github.com/FFmpeg/FFmpeg/blob/8f4702a93f87f9f76563e80f1ae2141a40029d9d/libavformat/movenc.c#L1747-L1775
>>  would then be applied to all movs of standard HD and SD size and cause many 
>> ffmpeg outputs to appear differently after the change.
> 
> 100% agree with this. We *should not* write guessed color information based
> off of resolution by default.
> 
> Further, if no color information is present, no colr atom should be written
> at all, unless it is forced by the user (aka the write_colr option right
> now).

TN2162 requires a colr atom for uncompressed yuv (including v210, v308, v408, 
etc) in mov, so I'd prefer to write it in this case. Note that the colr atom 
provides an option for unspecified for each of the color values, so there's a 
method to write a colr atom which basically says ¯\_(ツ)_/¯.

>> Could the guesswork here be removed? Or could write_colr be enabled with the 
>> option removed while the guesswork itself has its own option?
> 
> If the quesswork is removed or put under an option, I still maintain no
> colr atom should be written if no color information is known. Doing
> so is just asking for trouble, in my opinion.

I disagree. I'd prefer to follow the specification and write a colr atom (in 
the case of uncompressed yuv in mov) that say the colr is unspecified rather 
than to write no colr atom at all and create an invalid file. See 
https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG9.

I do agree that guesswork should be avoided or provided under an option that 
users could opt into. I suppose my preference order from support to oppose for 
writing uncompressed yuv in mov would be:

1: write a colr atom in all cases (if unknown, use unspecified values in colr 
atom)
2: write a colr atom if color data the known (no colr atom if unknown)
3: write a colr atom in all cases (if unknown, just make stuff up, #yolo)

Dave Rice

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Derek Buitenhuis]

On 11/20/2017 2:50 PM, Dave Rice wrote:
> I am hesitant to see write_colr option removed since the guesswork used here 
> https://github.com/FFmpeg/FFmpeg/blob/8f4702a93f87f9f76563e80f1ae2141a40029d9d/libavformat/movenc.c#L1747-L1775
>  would then be applied to all movs of standard HD and SD size and cause many 
> ffmpeg outputs to appear differently after the change.

100% agree with this. We *should not* write guessed color information based
off of resolution by default.

Further, if no color information is present, no colr atom should be written
at all, unless it is forced by the user (aka the write_colr option right
now).

> Could the guesswork here be removed? Or could write_colr be enabled with the 
> option removed while the guesswork itself has its own option?

If the quesswork is removed or put under an option, I still maintain no
colr atom should be written if no color information is known. Doing
so is just asking for trouble, in my opinion.

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Dave Rice]

> On Nov 20, 2017, at 9:01 AM, Carl Eugen Hoyos  wrote:
> 
> 2017-11-20 2:24 GMT+01:00 James Almer  >:
>> On 11/18/2017 11:19 PM, Dave Rice wrote:
>>> From 41da5e48f8788b85dd7a382030bb2866c506cc03 Mon Sep 17 00:00:00 2001
>>> From: Dave Rice 
>>> Date: Sat, 18 Nov 2017 20:31:27 -0500
>>> Subject: [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv 
>>> in
>>> mov
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>> 
>>> As required by Apple’s TN2162.
>>> ---
>>> libavformat/movenc.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>> index aaa1dedfd7..86960b19c1 100644
>>> --- a/libavformat/movenc.c
>>> +++ b/libavformat/movenc.c
>>> @@ -1978,7 +1978,7 @@ static int mov_write_video_tag(AVIOContext *pb, 
>>> MOVMuxContext *mov, MOVTrack *tr
>>> else
>>> av_log(mov->fc, AV_LOG_WARNING, "Not writing 'gama' atom. 
>>> Format is not MOV.\n");
>>> }
>>> -if (mov->flags & FF_MOV_FLAG_WRITE_COLR) {
>>> +if (mov->flags & FF_MOV_FLAG_WRITE_COLR || uncompressed_ycbcr) {
>>> if (track->mode == MODE_MOV || track->mode == MODE_MP4)
>>> mov_write_colr_tag(pb, track);
>>> else
>>> 
>> 
>> The write_colr option says "Write colr atom (Experimental, may be
>> renamed or changed, do not use from scripts)". Does that still apply? Is
>> the feature/spec still experimental?
>> 
>> If not, then the option and flag could be removed as well as part of
>> this patch.
> 
> I believe it should be removed in a follow-up patch.

I am hesitant to see write_colr option removed since the guesswork used here 
https://github.com/FFmpeg/FFmpeg/blob/8f4702a93f87f9f76563e80f1ae2141a40029d9d/libavformat/movenc.c#L1747-L1775
 would then be applied to all movs of standard HD and SD size and cause many 
ffmpeg outputs to appear differently after the change.

Could the guesswork here be removed? Or could write_colr be enabled with the 
option removed while the guesswork itself has its own option?
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Carl Eugen Hoyos]

2017-11-20 2:24 GMT+01:00 James Almer :
> On 11/18/2017 11:19 PM, Dave Rice wrote:
>> From 41da5e48f8788b85dd7a382030bb2866c506cc03 Mon Sep 17 00:00:00 2001
>> From: Dave Rice 
>> Date: Sat, 18 Nov 2017 20:31:27 -0500
>> Subject: [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in
>>  mov
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> As required by Apple’s TN2162.
>> ---
>>  libavformat/movenc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index aaa1dedfd7..86960b19c1 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -1978,7 +1978,7 @@ static int mov_write_video_tag(AVIOContext *pb, 
>> MOVMuxContext *mov, MOVTrack *tr
>>  else
>>  av_log(mov->fc, AV_LOG_WARNING, "Not writing 'gama' atom. 
>> Format is not MOV.\n");
>>  }
>> -if (mov->flags & FF_MOV_FLAG_WRITE_COLR) {
>> +if (mov->flags & FF_MOV_FLAG_WRITE_COLR || uncompressed_ycbcr) {
>>  if (track->mode == MODE_MOV || track->mode == MODE_MP4)
>>  mov_write_colr_tag(pb, track);
>>  else
>>
>
> The write_colr option says "Write colr atom (Experimental, may be
> renamed or changed, do not use from scripts)". Does that still apply? Is
> the feature/spec still experimental?
>
> If not, then the option and flag could be removed as well as part of
> this patch.

I believe it should be removed in a follow-up patch.

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Michael Niedermayer]

On Sun, Nov 19, 2017 at 10:24:41PM -0300, James Almer wrote:
> On 11/18/2017 11:19 PM, Dave Rice wrote:
> > From 41da5e48f8788b85dd7a382030bb2866c506cc03 Mon Sep 17 00:00:00 2001
> > From: Dave Rice 
> > Date: Sat, 18 Nov 2017 20:31:27 -0500
> > Subject: [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv 
> > in
> >  mov
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> > 
> > As required by Apple’s TN2162.
> > ---
> >  libavformat/movenc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > index aaa1dedfd7..86960b19c1 100644
> > --- a/libavformat/movenc.c
> > +++ b/libavformat/movenc.c
> > @@ -1978,7 +1978,7 @@ static int mov_write_video_tag(AVIOContext *pb, 
> > MOVMuxContext *mov, MOVTrack *tr
> >  else
> >  av_log(mov->fc, AV_LOG_WARNING, "Not writing 'gama' atom. 
> > Format is not MOV.\n");
> >  }
> > -if (mov->flags & FF_MOV_FLAG_WRITE_COLR) {
> > +if (mov->flags & FF_MOV_FLAG_WRITE_COLR || uncompressed_ycbcr) {
> >  if (track->mode == MODE_MOV || track->mode == MODE_MP4)
> >  mov_write_colr_tag(pb, track);
> >  else
> > 
> 
> The write_colr option says "Write colr atom (Experimental, may be
> renamed or changed, do not use from scripts)". Does that still apply? Is
> the feature/spec still experimental?
> 
> If not, then the option and flag could be removed as well as part of
> this patch.

hmm, ill wait with applying this patch until this is decided

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[James Almer]

On 11/18/2017 11:19 PM, Dave Rice wrote:
> From 41da5e48f8788b85dd7a382030bb2866c506cc03 Mon Sep 17 00:00:00 2001
> From: Dave Rice 
> Date: Sat, 18 Nov 2017 20:31:27 -0500
> Subject: [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in
>  mov
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> As required by Apple’s TN2162.
> ---
>  libavformat/movenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index aaa1dedfd7..86960b19c1 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -1978,7 +1978,7 @@ static int mov_write_video_tag(AVIOContext *pb, 
> MOVMuxContext *mov, MOVTrack *tr
>  else
>  av_log(mov->fc, AV_LOG_WARNING, "Not writing 'gama' atom. Format 
> is not MOV.\n");
>  }
> -if (mov->flags & FF_MOV_FLAG_WRITE_COLR) {
> +if (mov->flags & FF_MOV_FLAG_WRITE_COLR || uncompressed_ycbcr) {
>  if (track->mode == MODE_MOV || track->mode == MODE_MP4)
>  mov_write_colr_tag(pb, track);
>  else
> 

The write_colr option says "Write colr atom (Experimental, may be
renamed or changed, do not use from scripts)". Does that still apply? Is
the feature/spec still experimental?

If not, then the option and flag could be removed as well as part of
this patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Michael Niedermayer]

On Sun, Nov 19, 2017 at 10:05:56AM +0100, Reto Kromer wrote:
> Dave Rice wrote:
> 
> >As required by Apple’s TN2162.
> >---
> > libavformat/movenc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> LGTM

will apply

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn


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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Reto Kromer]

Dave Rice wrote:

>As required by Apple’s TN2162.
>---
> libavformat/movenc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

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