Quite a while ago, the specification was modified to change the way the nondefault scan format was signalled; this patch fixes that. Index: liboggz/dirac.c ==================================================================--- liboggz/dirac.c (revision 3759) +++ liboggz/dirac.c (working copy) @@ -122,6 +122,10 @@ 1, 9, 10, 9, 10, 9, 10, 4, 3, 7, 6, 4, 3, 7, 6, 2, 2, 7, 6, 7, 6, }; + static const int dirac_top_field_first[] = { /* extracted from table C.1 */ + 0, 0, 1, 0, 1, 0, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + }; + static const struct { ogg_uint32_t width, height; } dirac_fsize_tbl[] = { /* table 10.3 framesize */ @@ -150,11 +154,12 @@ if (dirac_bool( &bs )) { info->chroma_format = dirac_uint( &bs ); /* chroma_format */ } + if (dirac_bool( &bs )) { - info->interlaced = 0; - if (dirac_bool( &bs )) { /* interlaced */ - info->interlaced = 1; - info->top_field_first = dirac_bool( &bs ); /* top_field_first */ + int i = dirac_uint( &bs ); /* scan_format */ + if (i < 2) { + info->interlaced = i; + info->top_field_first = dirac_top_field_first[video_format]; } }
On Tue, Nov 4, 2008 at 12:56 AM, David Flynn <davidf+nntp at woaf.net> wrote:> Quite a while ago, the specification was modified to change the > way the nondefault scan format was signalled; this patch fixes that.Thanks for the patch. I've tried to fix things as discussed on irc. Please review r3762. Index: /liboggz/trunk/src/liboggz/dirac.c ==================================================================--- /liboggz/trunk/src/liboggz/dirac.c (revision 3665) +++ /liboggz/trunk/src/liboggz/dirac.c (revision 3762) @@ -116,9 +116,16 @@ {1,1}, /* this first value is never used */ {24000,1001}, {24,1}, {25,1}, {30000,1001}, {30,1}, - {50,1}, {60000,1001}, {60,1}, {15000,1001}, {25,2}, + {50,1}, {60000,1001}, {60,1}, {15000,1001}, {25,2} }; static const ogg_uint32_t dirac_vidfmt_frate[] = { /* table C.1 */ - 1, 9, 10, 9, 10, 9, 10, 4, 3, 7, 6, 4, 3, 7, 6, 2, 2, 7, 6, 7, 6, + 1, 9, 10, 9, 10, 9, 10, 4, 3, 7, 6, 4, 3, 7, 6, 2, 2, 7, 6, 7, 6 + }; + + static const int dirac_source_sampling[] = { /* extracted from table C.1 */ + 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0 + }; + static const int dirac_top_field_first[] = { /* from table C.1 */ + 0, 0, 1, 0, 1, 0, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 }; @@ -151,11 +158,17 @@ info->chroma_format = dirac_uint( &bs ); /* chroma_format */ } - if (dirac_bool( &bs )) { - info->interlaced = 0; - if (dirac_bool( &bs )) { /* interlaced */ - info->interlaced = 1; - info->top_field_first = dirac_bool( &bs ); /* top_field_first */ + + if (dirac_bool( &bs )) { /* custom_scan_format_flag */ + int scan_format = dirac_uint( &bs ); /* scan_format */ + if (scan_format < 2) { + info->interlaced = scan_format; + } else { /* other scan_format values are reserved */ + info->interlaced = 0; } + } else { /* no custom scan_format, use the preset value */ + info->interlaced = dirac_source_sampling[video_format]; } + /* field order is set by video_format and cannot be custom */ + info->top_field_first = dirac_top_field_first[video_format]; info->fps_numerator dirac_frate_tbl[dirac_vidfmt_frate[video_format]].fps_numerator;
On 2008-11-04, Ralph Giles <giles at xiph.org> wrote:> On Tue, Nov 4, 2008 at 12:56 AM, David Flynn <davidf+nntp at woaf.net> wrote: > >> Quite a while ago, the specification was modified to change the >> way the nondefault scan format was signalled; this patch fixes that. > > Thanks for the patch. I've tried to fix things as discussed on irc. > Please review r3762.That looks fine. ..david> - {50,1}, {60000,1001}, {60,1}, {15000,1001}, {25,2}, > + {50,1}, {60000,1001}, {60,1}, {15000,1001}, {25,2}Interesting -- i usually leave the final `,' there so that any future patch that appends something to the list doesn't have to modify the previous line to insert the ',' ..david
Ralph Giles
2008-Nov-04 23:10 UTC
[ogg-dev] Fwd: [PATCH] liboggz: Fix Dirac bitstream parsing
On Tue, Nov 4, 2008 at 2:41 PM, David Flynn <davidf+nntp at woaf.net> wrote:>> - {50,1}, {60000,1001}, {60,1}, {15000,1001}, {25,2}, >> + {50,1}, {60000,1001}, {60,1}, {15000,1001}, {25,2} > > Interesting -- i usually leave the final `,' there so that any future > patch that appends something to the list doesn't have to modify the > previous line to insert the ','Good point about patches. I just though it was for editing convenience. I've been broken of that by (ancient) compilers who are confused by trailing commas. The file was inconsistent so I normalized it in favour of my personal habit. For liboggz I don't think it's important: people tend not to build media software on such machines. So we can put them back if you want. -r
Seemingly Similar Threads
- [PATCH] liboggz: Fix Dirac bitstream parsing
- [PATCH 00/10] Add support for MPEG2 and VC-1 on VP3/VP4 for NV98-NVAF
- State Based vs. Behaviour Based Spec/Testing
- correlation matrix difference
- Suggestion to extend aggregate() to return multiple and/or named values