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

[Michel Bardiaux]

Michael Niedermayer wrote:
> Hi
> 
> On Wed, Feb 07, 2007 at 06:07:05PM +0100, Michel Bardiaux wrote:
>> Michel Bardiaux wrote:
>>> Michael Niedermayer wrote:
[snip]
>>>> 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 ...

As I read it, the MS spec does not make any exception about FACT being 
mandatory. To me it means WAV is simply not intended to be streamed, and 
  any attempt to stream a WAV should simply be killed at the very start. 
Unless someone can show me an example where a WAV (non-PCM) is streamed?

Anyway, the existing code already used start_tag/end_tag for the 'fmt ' 
chunk, at that uses url_fseek.

So my feeling is that is is simply not possible to stream a WAV 
correctly, at least not with the current implementation.

[snip]

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

You're the one who proposed an assert for that, see at top!

BTW I feel that the 'assert' should be hunted down even more ruthlessly 
as the dprintf. What do you think?

Greetings,
-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/