[FFmpeg-devel] [PATCH] MPEG sequeunce display extension parsing off by 2 error

[Michael Niedermayer]

On Mon, Jun 28, 2010 at 10:29:48PM -0400, Daniel Kristjansson wrote:
> On Tue, 2010-06-29 at 01:28 +0200, Michael Niedermayer wrote:
> > On Mon, Jun 28, 2010 at 01:09:02PM -0400, Daniel Kristjansson wrote:
> > > 
> > > This isn't a very important patch since these are trailing bits,
> > > but if we're going to have a skip_bits call it should be skipping
> > > the correct number of bits.
> > > -    skip_bits(&s->gb, 1); //marker
> > > +    skip_bits(&s->gb, 3); //marker
> > my copy of the spec doesnt contain 3 marker bits
> > could you quote the relevant part of the spec please (including
> > which exact version that is)?
> 
> 13818-2: 1995(E), p33. 

mine says 1995 too and it doesnt say draft (which doesnt mean that it
isnt one ...), can you confirm that yours is not a draft?

more precissely mines says:
6.2.2.4     Sequence display extension

|sequence_display_extension() {                |No. of  |Mnemonic|
|                                              |bits    |        |
| extension_start_code_identifier              |4       |uimsbf  |
| video_format                                 |3       |uimsbf  |
| colour_description                           |1       |uimsbf  |
| if ( colour_description ) {                  |        |        |
|  colour_primaries                            |8       |uimsbf  |
|  transfer_characteristics                    |8       |uimsbf  |
|  matrix_coefficients                         |8       |uimsbf  |
| }                                            |        |        |
| display_horizontal_size                      |14      |uimsbf  |
| marker_bit                                   |1       |bslbf   |
| display_vertical_size                        |14      |uimsbf  |
| next_start_code()                            |        |        |
|}                                             |        |        |



> They are not actually marker bits, they are
> undefined bits, so they do not have to be set like the bit between
> the horizontal and vertical size.

that means your patch is wrong. (more precissly the comment is)

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

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100629/cf28d6f8/attachment.pgp>