[FFmpeg-devel] [PATCH 1/3] mpeg12dec: validate color space

[Andreas Cadhalpun]

On 07.01.2017 13:44, Ronald S. Bultje wrote:
> On Fri, Jan 6, 2017 at 9:35 PM, Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
>> On Fri, Jan 06, 2017 at 09:43:24PM +0100, Andreas Cadhalpun wrote:
>>> On 23.12.2016 00:57, Andreas Cadhalpun wrote:
>>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>>> ---
>>>>  libavcodec/mpeg12dec.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>>>> index 63979079c8..d3dc67ad6a 100644
>>>> --- a/libavcodec/mpeg12dec.c
>>>> +++ b/libavcodec/mpeg12dec.c
>>>> @@ -1470,6 +1470,10 @@ static void mpeg_decode_sequence_display_extension(Mpeg1Context
>> *s1)
>>>>          s->avctx->color_primaries = get_bits(&s->gb, 8);
>>>>          s->avctx->color_trc       = get_bits(&s->gb, 8);
>>>>          s->avctx->colorspace      = get_bits(&s->gb, 8);
>>>> +        if (!av_color_space_name(s->avctx->colorspace)) {
>>>> +            av_log(s->avctx, AV_LOG_WARNING, "Invalid color space %d,
>> setting to unspecified\n", s->avctx->colorspace);
>>>> +            s->avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
>>>> +        }
>>>>      }
>>>>      w = get_bits(&s->gb, 14);
>>>>      skip_bits(&s->gb, 1); // marker
>>>>
>>>
>>> Ping for the series.
>>
>> i have no real objection to it.
>> iam used to see these being exported unchanged though so it feels a
>> bit odd
> 
> 
> Doesn't this destroy exporting of newly added colorspaces that have no
> explicit mention yet in libavutil? I'm not 100% sure this is a good thing.

Yes. That's how it is already done in libavcodec/h264_ps.c, so it just
makes handling unknown values consistent.

Best regards,
Andreas