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

[Michael Niedermayer]

On Tue, May 06, 2008 at 04:31:35PM +0200, Baptiste Coudurier wrote:
> 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.

mpeg-ps/ts supports arbitrary large positiv and negative timestamps, as it
just stores the 33 LSBs it really dont matter what is before them.


> 
> 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.

put_timestamp() will truncate them IIRC


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

yes probably, but dont forget murphies law ...


> 
> 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 ?

lazyness and i do not know for certain that they are useless for all muxers.


> 
> 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 ?

Ive not doubted that this has to be set to 3. The question is if
48khz/22khz AAC is allowed in flv or not. If one takes the spec literally
then the awnser is clearly no. But your muxer would store them blindly
with a claimed sample rate of 44khz.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080506/e0548e3b/attachment.pgp>