[Ffmpeg-devel] [PATCH] flacenc - md5

[Justin Ruggles]

Michael Niedermayer wrote:
> Hi
> 
> On Sat, Jul 08, 2006 at 10:41:05PM -0400, Justin Ruggles wrote:
> 
>>sheesh. I do need my cola tonight. The regression references would need
>>to be updated as well.  Here is a patch with regression updates.
> 
> 
> all things you do in the flac muxer will be missing if flac is stored in
> other containers, is raw flac more common then flac in ogg?
> 
> the problem i have with the changes is that theres lots of new code
> but the md5 will still be missing if flac is stored in any container
> furthermore the md5 isnt really mandatory so unless you can find a
> simple way to set the md5 which then also works with other containers,
> iam against setting it at all in lav*

Raw flac is not just more common, it is vastly more common.  I have only
seen a few ogg-flac files out there, but raw flac is all over the place.
 Like I said before, the reference encoder doesn't even put the MD5 in
ogg-flac files.  I think writing it, even if only in raw flac, is still
important.

Concerning trying to get it into other containers though...maybe your
previous suggestion of a 2-pass solution would work ok.  I would have to
study the current system for reading/writing the 2-pass info file to
know exactly how this could work though.  If it works how I think, it
might be the simplest solution & could avoid passing the md5 through
AVCodecContext or modifying the extradata after it has been written already.

As far as the other metadata, Ogg has to do things a bit differently
anyway.  The standard flac-to-ogg mapping adds its own header to the
streaminfo, then puts each metadata block into a separate ogg packet.

I could shrink & simplify the code quite a bit if the extradata is
explicitly defined to contain only the streaminfo.  In fact, I like this
idea.  It may limit some of the raw flac functionality, but it would be
simpler for other containers since they would know exactly what they are
writing.  I am still in favor of adding the padding and vorbis comment
in raw flac though.  The vorbis comment writing could conceivably be put
in a place where other muxers could access it if they wanted to...

> 
> [...]
> 
>>    // keep current count of audio samples
>>    if(pkt->pts != AV_NOPTS_VALUE) {
>>        flac->sample_count = (float)(pkt->pts+pkt->duration) *
>>                             (float)st->time_base.num /
>>                             (float)st->time_base.den *
>>                             (float)st->codec->sample_rate + 0.5;
> 
> 
> use av_rescale(), floats are unacceptable and inaccurate

ok. I'll try it and see if I can get it to work. Doing the integer math
directly, I kept getting 1 sample too short due to rounding of the pts
and duration. Could there be a way of setting a better time_base to
ensure single-sample accuracy?

> 
> 
>>Index: ffmpeg.c
>>===================================================================
>>--- ffmpeg.c	(revision 5683)
>>+++ ffmpeg.c	(working copy)
>>@@ -1393,13 +1393,11 @@
>>                             ret = 0;
>>                             /* encode any samples remaining in fifo */
>>                             if(fifo_bytes > 0 && enc->codec->capabilities & CODEC_CAP_SMALL_LAST_FRAME) {
>>-                                int fs_tmp = enc->frame_size;
>>                                 enc->frame_size = fifo_bytes / (2 * enc->channels);
>>                                 if(fifo_read(&ost->fifo, (uint8_t *)samples, fifo_bytes,
>>                                         &ost->fifo.rptr) == 0) {
>>                                     ret = avcodec_encode_audio(enc, bit_buffer, bit_buffer_size, samples);
>>                                 }
>>-                                enc->frame_size = fs_tmp;
>>                             }
>>                             if(ret <= 0) {
>>                                 ret = avcodec_encode_audio(enc, bit_buffer, bit_buffer_size, NULL);
> 
> 
> this change seems unrelated ...
> 
> [...]

It is related to the calculation of the sample count.  The last packet
was reporting an incorrect duration due to frame_size being set back to
the larger size.  The result was that the sample count was too high.
Leaving frame_size the actual size of the last frame seemed okay to me,
especially since it is the last frame anyway.


-Justin