[ogg-dev] libfishsound: Bug in fish_sound_flush

[Bob Ingraham]

> On 21 May 2013 00:17, Bob Ingraham <bobi at ingrahams.us> wrote:
>> I apologize if this is the wrong list for this, but after searching the
>> available lists at xiph.org, this is the closest I could find.
>>
>> I ran into a bug in the example fishsound-encode.c program where it gets
>> OGGZ_ERR_BAD_SERIALNO errors from oggz:
>>
>>   fish_sound_flush (fsound);
>>   oggz_run (oggz);
>>
>>   oggz_close (oggz);
>>
>>   fish_sound_delete (fsound);
>>
>> After debugging, it appears that fish_sound_flush() doesn't actually
>> flush
>> anything from the codec, so the succeeding oggz_run() doesn't write the
>> final packets to the oggz queue.
>>
>> Then, the oggz_close() is executed.
>>
>> However, the call to fish_sound_delete() actually generates the
>> remaining
>> packets and tries to write them to the closed oggz handle which
>> generates
>> the OGGZ_ERR_BAD_SERIALNO error for each packet.
>>
>> By moving the fish_sound_delete() ahead of the oggz_close(), all works
>> as
>> expected:
>>
>>   fish_sound_flush (fsound);
>>
>>   fish_sound_delete (fsound);
>>
>>   oggz_run (oggz);
>>
>>   oggz_close (oggz);
>>
>> So, it looks like fish_sound_delete() is actually flushing the remaining
>> packets whereas fish_sound_flush() is not.
>>
>> I am using libfishsound-1.0.0.
>>
>> Is this a bug?
>
> Hi,
>
> the documentation is certainly lacking (it just says "flush encoded
> buffers"). Actually fish_sound_flush() only exists for the speex codec
> (it is a no-op for vorbis). I can't recall the use-case but I think it
> is for use when someone stops talking, to ensure the recently-spoken
> words are sent rather than waiting to fill the buffer with silence.
>
> Likewise, the documentation for fish_sound_delete() just says "Delete
> a FishSound object" whereas it really should say "perform any one-time
> finalizations, which may produce encoded data, and delete ...". The
> API is designed that way so that you can't attempt to do vorbis
> finalization mid-stream.
>
> So for your vorbis code, there's no need to call fish_sound_flush() at
> all, and oggz_close() should always be called last.
>
> hope that helps,
>
> Conrad.

Actually, that helps a great deal - thank-you so much.

Since I am in the midst of learning-and-implementing as I go, perhaps I
could be of assistance in updating the documentation and fixing the
example apps?

I think it's the least I can do for gaining the benefit of such a useful
set of libraries for ogg/vorbis/flac/speex.


Speaking of which, there is one other "bug" in the fishsound-encode.c
example program:

- It doesn't set the EOS flag on the final encoded ogg packet in the
output stream.

And, after scouring the documentation, I cannot find an obvious, reliable
way of appropriately setting the EOS flag using existing fishsound/oggz
calls.

I mean, yes, I know it needs to be set in the Ogg packet (op) structure in
the FishSoundEncoded callback - the problem is knowing WHEN you are
processing the last packet from the encoder.

Any thoughts on how to solve this?

Cheers,
Bob Ingraham