[Ffmpeg-cvslog] Re: r7150 - trunk/libavformat/matroska.c

[Steve Lhomme]

Steve Lhomme wrote:
> Aurelien Jacobs wrote:
>> On Wed, 22 Nov 2006 01:19:26 +0100
>> Michael Niedermayer <michaelni at gmx.at> wrote:
>>
>>> Hi
>>>
>>> On Wed, Nov 22, 2006 at 12:47:21AM +0100, aurel wrote:
>>>> Author: aurel
>>>> Date: Wed Nov 22 00:47:21 2006
>>>> New Revision: 7150
>>>>
>>>> Modified:
>>>>    trunk/libavformat/matroska.c
>>>>
>>>> Log:
>>>> add support for FLAC in matroska
>>>>
>>>> Modified: trunk/libavformat/matroska.c
>>>> ============================================================================== 
>>>>
>>>> --- trunk/libavformat/matroska.c    (original)
>>>> +++ trunk/libavformat/matroska.c    Wed Nov 22 00:47:21 2006
>>>> @@ -224,6 +224,7 @@
>>>>      {"A_DTS"            , CODEC_ID_DTS},
>>>>      {"A_VORBIS"         , CODEC_ID_VORBIS},
>>>>      {"A_AAC"            , CODEC_ID_AAC},
>>>> +    {"A_FLAC"           , CODEC_ID_FLAC},
>>>>      {"A_WAVPACK4"       , CODEC_ID_WAVPACK},
>>>>      {"A_TTA1"           , CODEC_ID_TTA},
>>>>      {NULL               , CODEC_ID_NONE}
>>>> @@ -2265,6 +2266,14 @@
>>>>                  }
>>>>              }
>>>>  
>>>> +            else if (codec_id == CODEC_ID_FLAC) {
>>>> +                AVPacket *pkt = av_mallocz(sizeof(AVPacket));
>>>> +                av_new_packet(pkt, track->codec_priv_size);
>>>> +                memcpy(pkt->data, track->codec_priv, 
>>>> track->codec_priv_size);
>>>> +                matroska_queue_packet(matroska, pkt);
>>>> +                track->codec_priv_size = 0;
>>>> +            }
>>> hmm, are you injecting a global header as first packet into the stream?
>>
>> Indeed I am. Well not exactly in fact...
>> FLAC uses a 34 bytes extradata struct. The flac decoder accept either:
>>  - 34 bytes in the extradata then normal data in normal packets
>>    (with a 0xFFF8 marker prepended to the first data packet).
>>  - 34 bytes prepended to the first normal data packet then normal
>>    data packets.
>> Ok, so now as you know that matroska is quite braindead, you don't even
>> expect it to contain a proper 34 bytes extradata and normal data packets.
>> You may at least expect that they stored the extradata prepended to
>> the first data packet ?
>> They managed to do even worse ! They indeed pack the extradata with the
>> first data packet, but they store it in the extradata block !
> 
> If that's the case it's ugly. Although it may be due to libflac at that 
> time that needed the extradata+1 frame to work correctly (keep in mind 
> this stuff is old). But that's just a supposition.
> 
> Hopefully Mosu or Haali know more about that.

Just been reading that page: 
http://www.matroska.org/technical/specs/codecid/

"A_FLAC	FLAC (Free Lossless Audio Codec)	The private data contains all 
the header/metadata packets before the first data packet. These include 
the first header packet containing only the word fLaC as well as all 
metadata packets."

It doesn't say that it contains data packet. It actually says it 
doesn't. If it does in your file, your file is incorrect. (could be a 
bug in mkmverge).

Steve