[Ffmpeg-devel] [DRAFT] Add fact chunk to non-PCM wav

[Michael Niedermayer]

Hi

On Wed, Feb 07, 2007 at 06:07:05PM +0100, Michel Bardiaux wrote:
> Michel Bardiaux wrote:
> >Michael Niedermayer wrote:
> >>Hi
> >>
> >>On Thu, Nov 09, 2006 at 01:53:08PM +0100, Michel Bardiaux wrote:
> >>>Michael Niedermayer wrote:
> >>>>Hi
> >>>>
> >>>>On Thu, Nov 09, 2006 at 11:36:13AM +0100, Michel Bardiaux wrote:
> >>>>>Michael has reported that
> >>>>>
> >>>>>>>interresting, accoridng to microsofts excelent and unambigous 
> >>>>>>>documentation
> >>>>>>>(not kidding ...)
> >>>>>>>   Fact Chunk
> >>>>>>>This chunk is required for all WAVE formats other than 
> >>>>>>>WAVE_FORMAT_PCM. It stores file
> >>>>>>>dependent information about the contents of the WAVE data. It 
> >>>>>>>currently specifies the time length of the
> >>>>>>>data in samples.
> >>>>>>>
> >>>>>>>so this must not be under CODEC_ID_MSGSM, also it must be a 
> >>>>>>>seperate patch
> >>>>>>>as its not CODEC_ID_MSGSM specific
> >>>>>I took this literally, hence CODEC_ID_PCM_ALAW and 
> >>>>>CODEC_ID_PCM_MULAW will get a fact chunk too.
> >>>>:)
> >>>>
> >>>>now just take the largest pts minus the smallest pts of any packet 
> >>>>stored
> >>>>and convert that by using AVStream.time_base and 
> >>>>AVCodecContext.sample_rate
> >>>>to the number of samples and store that in the fact chunk
> >>>Right, the patch does not make sense without the final update of the 
> >>>chunk, and with a *general* formula, not the one I had specialised 
> >>>for MSGSM. I'm rather rusty on the handling of timestamps, so this 
> >>>might take some time 
> >>
> >>in the write packet function
> >>assert(avpacket->pts != AV_NOPTS_VALUE);
> >>context->maxpts= FFMAX(context->maxpts, avpacket->pts);
> >>context->minpts= FFMIN(context->minpts, avpacket->pts);
> >>
> >>and in write_trailer
> >>number_of_sample= av_rescale(context->maxpts - context->minpts, 
> >>avctx->sample_rate * (int64_t)avstream->time_base.num, 
> >>avstream->time_base.den);
> >>
> >>untested of course but it should work approximately that way ...
> >
> >Thanks, it works, with a little correction: the duration of the last 
> >packet must be added to the difference in pts. I assumed the 
> >pkt->duration is in the same unit as the timestamps.
> >
> >>
> >>>>then run the regression tests and send a patch which updates the 
> >>>>checksums
> >>>Not clear how to proceed here, do you mean a subsequent patch, or in 
> >>>the same?
> >
> >I havent forgotten to make test, and of course the changes breaks 
> >regression since it adds 12 bytes to every non-PCM wav. I will post the 
> >final version with new checksums when the rest is approved.
> >
> Oops, forgot the attachment.
> 

[...]

> @@ -46,6 +49,12 @@
>      }
>      end_tag(pb, fmt);
>  
> +    if(s->streams[0]->codec->codec_tag != 0x01 /* hence for all other than PCM */) {
> +        fact = start_tag(pb, "fact");
> +        put_le32(pb, 0);
> +        end_tag(pb, fact);
> +    }

the fact chunk should be ommited if url_is_streamed() as we wont be able to
fill it later ...


> +
>      av_set_pts_info(s->streams[0], 64, 1, s->streams[0]->codec->sample_rate);
>  
>      /* data header */
> @@ -59,7 +68,15 @@
>  static int wav_write_packet(AVFormatContext *s, AVPacket *pkt)
>  {
>      ByteIOContext *pb = &s->pb;
> +    WAVContext *wav = s->priv_data;
>      put_buffer(pb, pkt->data, pkt->size);
> +    if(pkt->pts == AV_NOPTS_VALUE) {
> +        av_log(s, AV_LOG_ERROR, "wav_write_packet: NOPTS\n");
> +        return -1;
> +    }

hmm, this is a little nasty behaviour for a missing pts value when muxing
in a format which doesnt really "use" pts ...
i would prefer if we just ignore such packets in the min/maxpts calculation


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070207/fdbc15aa/attachment.pgp>