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

[Paul B Mahol]

On 9/16/13, JULIAN GARDNER <joolzg at btinternet.com> wrote:
>
>
>>________________________________
>> From: Michael Niedermayer <michaelni at gmx.at>
>>To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
>>Sent: Monday, 16 September 2013, 18:09
>>Subject: Re: [FFmpeg-devel] Small modifcation to libavformat/dvbsubdec.c
>>
>>
>>On Mon, Sep 16, 2013 at 08:46:39AM +0100, JULIAN GARDNER wrote:
>>>
>>>
>>> >________________________________
>>> > From: JULIAN GARDNER <joolzg at btinternet.com>
>>> >To: FFmpegdevelopment discussions and patches <ffmpeg-devel at ffmpeg.org>
>>> >
>>> >Sent: Wednesday, 4 September 2013, 23:51
>>> >Subject: Small modifcation to libavformat/dvbsubdec.c
>>> >
>>> >
>>> >
>>> >diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
>>> >index 955925a..faa510a 100644
>>> >--- a/libavcodec/dvbsubdec.c
>>> >+++ b/libavcodec/dvbsubdec.c
>>> >@@ -1015,9 +1015,9 @@ static void
>>> > dvbsub_parse_clut_segment(AVCodecContext *avctx,
>>> >
>>> >         if (depth & 0x80)
>>> >             clut->clut4[entry_id] = RGBA(r,g,b,255 - alpha);
>>> >-        if (depth & 0x40)
>>> >+        else if (depth & 0x40)
>>> >             clut->clut16[entry_id] = RGBA(r,g,b,255 - alpha);
>>> >-        if (depth &
>>> 0x20)
>>> >+        else if (depth & 0x20)
>>> >             clut->clut256[entry_id] = RGBA(r,g,b,255 - alpha);
>>> >     }
>>> >     }
>>> >
>>> >
>>> >Just so it adheres to spec. Page 25 of ETS300743 V1.2.1, page 25.
>>> >
>>> >2-bit/entry_CLUT_flag: If set to '1', this indicates that this CLUT
>>> > value is to be loaded into the identified entry of the
>>> 2-bit/entry CLUT.
>>> >4-bit/entry_CLUT_flag: If set to '1', this indicates that this CLUT
>>> > value is to be loaded into the identified entry of the
>>> 4-bit/entry CLUT.
>>> >8-bit/entry_CLUT_flag: If set to '1', this indicates that this CLUT
>>> > value is to be loaded into the identified entry of the
>>> 8-bit/entry CLUT.
>>> >NOTE 1: Only one N-bit/entry_CLUT_flag shall be set to 1 per
>>> > CLUT_entry_id and its associated Y-, Cr-, Cb- and
>>> T -values.
>>> >joolz
>>> >
>>> Any comment on this, will it be pushed?
>>
>>what does this patch fix ?
>>please provide a testcase or if it fixes spec compliance a quote of
>>the spec saying that if multiple bits are set this new behavior is
>>to be used.
>>
>>You quote just a part of the spec that says that the affected case
>>shall not occur. why would a change in behavior for a case that lies
>>outside the spec be a matter of spec adherence ?
>>I surely am missing something here ...
>>
>>[...]
>>--
>>Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>>No human being will ever know the Truth, for even if they happen to say it
>>by chance, they would not even known they had done so. -- Xenophanes
>>
>>_______________________________________________
>>ffmpeg-devel mailing list
>>ffmpeg-devel at ffmpeg.org
>>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> This patch fixes a non corfmance to the DVB Subtitle spectications, as i
> pointed out in my mail.
>
> No i cannot provide a non working one as this would break the specification.
> All DVB Streams adhere to this specification, find me one that fails and I
> will bow to your interpretation.
>
>
> I think English is not you mother tongue but
>
>
> "NOTE 1: Only one N-bit/entry_CLUT_flag SHALL be set to 1 per CLUT_entry_id
> and its associated Y-, Cr-, Cb- and T -values."

That is mandatory for encoder, but does specification mention what
decoder should do
if it finds all bits set?

>
>
> SHALL (from dictionary.com)
> 3.(in laws, directives, etc.) must; is or are obliged to: The meetings of
> the council shall be public.

Usually SHALL and similar are explained in specification itself so
that one do not
need to look up at dictionary.

>
> OBLIGED (from dictionary.com)
> 1.to require or constrain, as by law, command, conscience, or force of
> necessity.
>
>
> CONSTRAIN (from dictionary.com)
> 1. to force, compel, or oblige
>
>
> To a native english speaker this is clear, You can only set a one of the 3
> bits.
>
>
> So its either ffmpeg follows spec, or ffmpeg follows Michaels interpretation
> of the spec.

Please refrain from such attacks.