flac dev - Nov 2006 - [PATCH] Re: Strangeness with OggFlac files

--- Erik de Castro Lopo <erikd-flac@mega-nerd.com>
wrote:
> Erik de Castro Lopo wrote:
> 
> > Josh et al,
> > 
> > I've been tracking down a problem with generating OggFlac files.
> 
> <snip>
> 
> > I have looked into this and suspect that the
> FLAC__stream_encoder_finish()
> > function needs to finish off the stream using a call to
> ogg_stream_flush()
> > instead of the standard function ogg_stream_pageout().
> 
> I bashed on this a bit further and came up with the following patch:
> 
>     http://www.mega-nerd.com/tmp/flac-end_of_stream.diff
> 
> There's probably a more tasteful way of acheiving the same result,
> but
> this version does fix the problem.
thanks Erik, can you confirm that this is happening only because
the total_samples_estimate is 0?  (so the e_o_s setting in the
existing code is never triggered).

Josh




 
____________________________________________________________________________________
Sponsored Link

Degrees online in as fast as 1 Yr - MBA, Bachelor's, Master's, Associate
Click now to apply http://yahoo.degrees.info

Josh Coalson wrote:
> thanks Erik, can you confirm that this is happening only because
> the total_samples_estimate is 0?  (so the e_o_s setting in the
> existing code is never triggered).
In the Ogg code, I believe that there are two things that must be 
satisfied to correctly write an end of stream:

   - packet.e_o_s must be true
   - Need to call ogg_stream_flush() instead of instead of 
     ogg_stream_pageout()

I haven't tested it, but I don't think just setting packet.e_o_s
is sufficient to close the stream. The forced ogg_stream_flush()
is required.

Maybe these two actions can be carried out in the function
FLAC__ogg_encoder_aspect_finish() which currently only does this:

   void FLAC__ogg_encoder_aspect_finish(FLAC__OggEncoderAspect *aspect)
   {
	(void)ogg_stream_clear(&aspect->stream_state);
	/*@@@ what about the page? */
   }

I could investigate this further, but I'm really busy over the next 
couple of days. I wouldn't be able to look at this until this weekend
or possibly the next.

Erik
-- 
+-----------------------------------------------------------+
  Erik de Castro Lopo
+-----------------------------------------------------------+
"Do I do everything in C++ and teach a course in advanced swearing?"
-- David Beazley at IPC8, on choosing a language for teaching

On Tue, Nov 07, 2006 at 06:31:04AM +1100, Erik de Castro Lopo wrote:
> I haven't tested it, but I don't think just setting packet.e_o_s
> is sufficient to close the stream. The forced ogg_stream_flush()
> is required.
libogg checks the e_o_s flag (and has always done so, according to svn)
so you shouldn't have to call ogg_stream_flush() except when you want to 
force a page boundary (such as after the header packets).

ogg/src/framing.c line 449

One common mistake it to not loop on ogg_stream_pageout() but it looks 
like ogg_encoder_aspect.c is doing that.

 -r

On Mon, Nov 06, 2006 at 08:57:49AM -0800, Josh Coalson wrote:
> thanks Erik, can you confirm that this is happening only because
> the total_samples_estimate is 0?  (so the e_o_s setting in the
> existing code is never triggered).
I can confirm this is the case.

 -r