[FFmpeg-devel] Small modifcation to libavformat/dvbsubdec.c

[Reimar Döffinger]

"Reimar Döffinger" <Reimar.Doeffinger at gmx.de> wrote:
>
>
>On 17.09.2013, at 08:06, JULIAN GARDNER <joolzg at btinternet.com> wrote:
>> ----- Original Message -----
>>> From: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>>> To: FFmpeg development discussions and patches
><ffmpeg-devel at ffmpeg.org>
>>> Cc: "ffmpeg-devel at ffmpeg.org" <ffmpeg-devel at ffmpeg.org>
>>> Sent: Tuesday, 17 September 2013, 7:26
>>> Subject: Re: [FFmpeg-devel] Small modifcation to
>libavformat/dvbsubdec.c
>>> 
>>> 
>>> 
>>> On 16.09.2013, at 23:58, Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:
>>> 
>>>> JULIAN GARDNER <joolzg <at> btinternet.com> writes:
>>>> 
>>>>> Currently in my own personal tree are
>>>>> 
>>>>> Complete
>>>>> 1. DVB Teletext language parsing
>>>>> 2. DVB Teletext SI generation for -c:s copy
>>>> 
>>>> Please send patches, or even better, setup a git 
>>>> clone and ask Michael to merge. I know that these 
>>>> features are very welcome!
>>>> 
>>>> I may (completely) misunderstand this thread but 
>>>> I suspect patches that make decoders more strict 
>>>> than absolutely required are generally not a 
>>>> good idea, particularly if no sample is known 
>>>> that profits from the change.
>>> 
>>> The patch doesn't make it more strict.
>>> A patch making it more strict would do something like adding
>>> if (!!(depth & 0x80) + !!(depth & 0x40) + !!(depth & 0x20) > 1)
>>>   return AVERROR_INVALID_DATA;
>>> 
>>> What it does instead is that it interprets an invalid value like
>0xc0 as 0x80.
>>> The spec (at least the quoted part) does not say a thing about how
>corrupt data 
>>> should be handled as far as I can tell, and 3 of us (at least 2 with
>extensive 
>>> experience reading and implementing specifications) agree on that,
>so while 
>>> experience is good and something to respect the reasoning for the
>change makes 0 
>>> sense to us so far.
>>> The commit message really should explain why a value of 0xc0 should
>be treated 
>>> like 0x80 and not e.g. 0x40 or 0x00, just as examples.
>> 
>> Spec: Only 1 bit SHALL be set. IS THIS NOT CLEAR? You are making
>something out of the spec that does not exist, dont make up stuff, use
>the spec as it is written.
>
>I suggest you try to read what I wrote with a calm mind, you are being
>_extremely_ insulting.

And just in case we really are idiots: maybe you can just give a specific example of a value of the depth variable where your patch fixes behaviour?
I am fairly certain we all understand what that sentence says, we just see it as confirming that your patch should not make a difference.