Re: [libav-devel] [PATCH] mpeg2enc: Don't mark all streams as component video

2017-08-13 Thread Mark Thompson
On 13/08/17 09:07, Anton Khirnov wrote:
> Quoting Mark Thompson (2017-08-12 23:16:13)
>> Since there is no information about the source format, "unspecified"
>> is the correct value to write here.
>> ---
>>  libavcodec/mpeg12enc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
>> index 103f3aaa7..406950901 100644
>> --- a/libavcodec/mpeg12enc.c
>> +++ b/libavcodec/mpeg12enc.c
>> @@ -297,7 +297,7 @@ static void mpeg1_encode_sequence_header(MpegEncContext 
>> *s)
>>  
>>  put_header(s, EXT_START_CODE);
>>  put_bits(>pb, 4, 2); // sequence 
>> display extension
>> -put_bits(>pb, 3, 0); // 
>> video_format: 0 is components
>> +put_bits(>pb, 3, 5); // 
>> video_format: 5 is unspecified
>>  put_bits(>pb, 1, 1); // 
>> colour_description
>>  put_bits(>pb, 8, s->avctx->color_primaries); // 
>> colour_primaries
>>  put_bits(>pb, 8, s->avctx->color_trc);   // 
>> transfer_characteristics
>> -- 
>> 2.11.0
> 
> Sounds reasonable. I hope it doesn't break any crappy decoders.

Crappy decoders which parse the sequence display extension and then do 
something with the result?  Seems unlikely.

In any case, this matches the behaviour of other encoders (x262 defaults to 5 
for MPEG-2, x264 defaults to 5 in the matching VUI field for H.264), and also 
the value the standard says the decoder has to assume if no sequence display 
extension is present.

- Mark
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] mpeg2enc: Don't mark all streams as component video

2017-08-13 Thread Anton Khirnov
Quoting Mark Thompson (2017-08-12 23:16:13)
> Since there is no information about the source format, "unspecified"
> is the correct value to write here.
> ---
>  libavcodec/mpeg12enc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> index 103f3aaa7..406950901 100644
> --- a/libavcodec/mpeg12enc.c
> +++ b/libavcodec/mpeg12enc.c
> @@ -297,7 +297,7 @@ static void mpeg1_encode_sequence_header(MpegEncContext 
> *s)
>  
>  put_header(s, EXT_START_CODE);
>  put_bits(>pb, 4, 2); // sequence 
> display extension
> -put_bits(>pb, 3, 0); // video_format: 
> 0 is components
> +put_bits(>pb, 3, 5); // video_format: 
> 5 is unspecified
>  put_bits(>pb, 1, 1); // 
> colour_description
>  put_bits(>pb, 8, s->avctx->color_primaries); // 
> colour_primaries
>  put_bits(>pb, 8, s->avctx->color_trc);   // 
> transfer_characteristics
> -- 
> 2.11.0

Sounds reasonable. I hope it doesn't break any crappy decoders.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel