[FFmpeg-devel] [PATCH] Matroska Muxer

[Aurelien Jacobs]

On Fri, 24 Aug 2007 12:45:03 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> Hi
> 
> On Thu, Aug 23, 2007 at 08:47:24PM -0400, David Conrad wrote:
> > On Aug 20, 2007, at 4:30 PM, David Conrad wrote:
> >
> >> On Aug 14, 2007, at 3:13 PM, Michael Niedermayer wrote:
> >>
> >>> Hi
> >>>
> >>> On Mon, Aug 13, 2007 at 08:38:25PM -0400, David Conrad wrote:
> >>
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * Calculate how many bytes are needed to represent a given size in 
> >>>> EBML
> >>>> + */
> >>>> +static int ebml_size_bytes(uint64_t size)
> >>>> +{
> >>>> +    int bytes = 1;
> >>>> +    while ((size+1) >> bytes*7) bytes++;
> >>>> +    return bytes;
> >>>> +}
> >>>>
> >>>
> >>> isnt ebml_size_bytes and ebml_id_size the same if the IDs would be stored
> >>> properly?
> >>> i mean currently the #define *_ID_* is in encoded form while size of 
> >>> course
> >>> cannot be, so if we would change the #defines to be in normal form then i
> >>> think this could allow some simplifications, though i might be wrong ...
> >>
> >> I think this is possible, the only caveat is that if IDs with all ones are 
> >> ever used (they're currently reserved) then this won't work for them. I'll 
> >> send a patch changing the demuxer, then change the muxer.
> >
> > Another thing I realized is that I'm pretty sure that an ID of 0x81 is 
> > considered different from an ID of 0x4001, but they're the same number if 
> > not in ebml form. I don't think there are any IDs right now that this is an 
> > issue for.
> >
> > Another problem is that the IDs are stored in encoded form in the seek head 
> > as unsigned integers (thus they need to be read by ebml_read_uint(), and 
> > they're compared to the #defines. I can't come up with a clean way to 
> > handle that in matroska_parse_seekhead() that doesn't completely negate the 
> > simplification to the muxer.
> >
> > Attached is the current difference for doing this for the muxer and 
> > demuxer, comments?
> 
> aurelian?

First, it's not aurelian, it's aurelien (really it Aur?lien, but I tend
to prefer ASCII). Feel free to simply write aurel to avoid any mistake ;-)

> are you ok with this, if not iam fine with leaving the code as
> is and use the normal #defines in the muxer  ...

I liked your idea, but I dislike the hack needed to read seek_id.
Moreover, all the #define *_ID_* are exactly what can be found in
the Matroska specs. I think it's easier to maintain this way.
So I prefer keeping the code as is.

Aurel