[FFmpeg-soc] [soc]: r209 - matroska/matroskaenc.c

[David Conrad]

On Jun 2, 2007, at 6:26 AM, Michael Niedermayer wrote:

> Hi
>
> On Fri, Jun 01, 2007 at 06:46:13AM +0200, conrad wrote:
>> Author: conrad
>> Date: Fri Jun  1 06:46:13 2007
>> New Revision: 209
>>
>> Log:
>> Beginning of mkv muxer, only EBML head is written correctly
> [...]
>> +    while (size >> (bytes*8 + 7-bytes) > 0) bytes++;
>
> while (size >> (bytes*7 + 7)) bytes++;

Changed.

> [...]
>> +// XXX: should we do any special checking for valid strings for  
>> these 2 functions?
>
> i would say no, or put the checks in generic lavf code, duplicating  
> string
> validity checks in every muxer is silly ...

Looking at the Matroska spec again, I only see only one string  
written as ASCII being user-specifiable in lavf right now, the ISO  
639-2 language code. The other user-specifiable strings would be  
written as UTF-8. I don't think the language code is being checked  
anywhere, but if it were, it probably should be against the official  
table of codes. If I were to write string validity checks for user- 
specified strings, where should they go in lavf?

>> +static void put_ebml_string(ByteIOContext *pb, unsigned int  
>> elementid, char *str)
>> +{
>> +    put_ebml_binary(pb, elementid, str, strlen(str));
>> +}
>> +
>> +static void put_ebml_utf8(ByteIOContext *pb, unsigned int  
>> elementid, char *str)
>> +{
>> +    put_ebml_binary(pb, elementid, str, strlen(str));
>> +}
>
> also why not just call put_ebml_binary() for these 2 directly?

I made the 2 different functions in case I added error checking  
later, so I agree at least one should be removed, though I kinda like  
having the implied strlen() for writing strings.

> [...]
>> +    ebml_header = start_ebml_master(pb, EBML_ID_HEADER);
>> +    put_ebml_uint(pb, EBML_ID_EBMLVERSION, 1);
>> +    put_ebml_uint(pb, EBML_ID_EBMLREADVERSION, 1);
>> +    put_ebml_uint(pb, EBML_ID_EBMLMAXIDLENGTH, 4);
>> +    put_ebml_uint(pb, EBML_ID_EBMLMAXSIZELENGTH, 8);
>> +    put_ebml_string(pb, EBML_ID_DOCTYPE, "matroska");
>> +    put_ebml_uint(pb, EBML_ID_DOCTYPEVERSION, 1);
>> +    put_ebml_uint(pb, EBML_ID_DOCTYPEREADVERSION, 1);
>
> they could be aligned for better readability like:
>
>
> put_ebml_uint   (pb, EBML_ID_EBMLVERSION        ,           1);
> put_ebml_uint   (pb, EBML_ID_EBMLREADVERSION    ,           1);
> put_ebml_uint   (pb, EBML_ID_EBMLMAXIDLENGTH    ,           4);
> put_ebml_uint   (pb, EBML_ID_EBMLMAXSIZELENGTH  ,           8);
> put_ebml_string (pb, EBML_ID_DOCTYPE            ,  "matroska");
> put_ebml_uint   (pb, EBML_ID_DOCTYPEVERSION     ,           1);
> put_ebml_uint   (pb, EBML_ID_DOCTYPEREADVERSION ,           1);

Done, I'll try to keep things aligned like this.