[FFmpeg-devel] [PATCH] h264/aac in flv

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Tue, May 06, 2008 at 03:42:40PM +0200, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
> [...]
>>> [...]
>>>> Index: libavformat/utils.c
>>>> ===================================================================
>>>> --- libavformat/utils.c	(revision 13010)
>>>> +++ libavformat/utils.c	(working copy)
>>>> @@ -2401,8 +2401,10 @@
>>>>  static void truncate_ts(AVStream *st, AVPacket *pkt){
>>>>      int64_t pts_mask = (2LL << (st->pts_wrap_bits-1)) - 1;
>>>>  
>>>> -//    if(pkt->dts < 0)
>>>> -//        pkt->dts= 0;  //this happens for low_delay=0 and B-frames, FIXME, needs further investigation about what we should do here
>>>> +    /* negative dts happens for low_delay=0 and B-frames
>>>> +     * must not truncate because wrap bits < 64 will not get them */
>>>> +    if(pkt->dts < 0)
>>>> +        return;
>>> Iam against this change.
>> Can you please give a reason ?
> 
> The code truncates timestamps to the number of bits requested by the muxer
> flv requests 32, it gets 32.
> Your code breaks that, as it returns the complete non truncated timestamp if
> it happens to be negative.
> This for example is not what mpeg-ps/ts requires. mpeg requires a truncated
> timestamp and a 64bit negative one just isnt correct for it.

Well mpeg-ps/ts does not support negative dts/pts in any way, and I
don't know ANY format supporting it.

It seem mpeg-ps muxer set wrap bits to 64, so what will happens when
pts/dts value needs more than 33 bits ? Code won't truncate them.

I think lavf has not much chance to get high pts/dts negative values,
while getting -1/-2(bpyramid) is pretty common.

It would be nice to fix this issue someday, there is a FIXME :>
I'm actually trying to right now ...

>> Every muxer not using 64 bit pts will have to cast to get the correct
>> value. This seems a bit odd.
> 
> If you want the "correct" value then use 64 instead of 32.

Well if wrapping bits is useless, why not removing it ?

Focusing on mpeg-ps is good, but I bet than FFmpeg is more used to
produce flv than mpeg-ps nowdays though ;)

>> Anyway patch updated.
>>
>>> [...]
>>>> @@ -49,11 +52,15 @@
>>>>      offset_t duration_offset;
>>>>      offset_t filesize_offset;
>>>>      int64_t duration;
>>>> +    int delay; ///< first dts delay for AVC
>>>>  } FLVContext;
>>>>  
>>>>  static int get_audio_flags(AVCodecContext *enc){
>>>>      int flags = (enc->bits_per_sample == 16) ? FLV_SAMPLESSIZE_16BIT : FLV_SAMPLESSIZE_8BIT;
>>>>  
>>>> +    if (enc->codec_id == CODEC_ID_AAC) // specs force these parameters
>>>> +        return FLV_CODECID_AAC | FLV_SAMPLERATE_44100HZ | FLV_SAMPLESSIZE_16BIT | FLV_STEREO;
>>> Is this also correct for AAC streams for which these arent true? Or are
>>> such streams just not supported?
>>>
>> Streams are supported (like mp3 48khz btw), and play well. Like written,
>> specs mandates these values.
> 
> I know but what about lets say 48khz AAC or 22khz AAC your code would mux this
> with a claimed samplerate of 44khz.
> Is such 22khz AAC and 48khz AAC legal in flv accoridng to spec or is just
> 44khz AAC allowed?
> If later then the muxer should reject AAC with samplerates differing from 
> 44khz.
> 

Rofl:
"Sampling rate
For AAC: always 3"

Is this ok for you ?

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA