[Flac] Re: Bug in ogg123 playback of OggFLAC files?

[Barry Bouwsma]

[I'm not online regularly, so don't include me in any replies.]
[ taken from the web-ified list archive, so References: header
  is missing the last Message-ID, sorry ]

Figuring the flac@ list is more appropriate, I move this from
the original location at vorbis@ to continue...

> > I used a recent ogg123 to play back a FLAC file I had created,
> > and saw it wasn't able to seek within the file.  So, I thought

> > That ogg123 wasn't able to recognize the file as something it

> >   if (len >= 32 && memcmp(buf, "OggS", 4) == 0
> >                 && memcmp(buf+28, "fLaC", 4) == 0)
> >     return 1; /* Ogg FLAC */

> also in FLAC 1.1.1 is the first official (i.e. correct)
> FLAC->Ogg mapping.  for more info about the history of it

Thanks for the pointer.  Having grabbed all the latest source a
couple days ago and built everything new, I shall now pretend to
have a better idea of what needs to be patched.

The vorbis-tools/ogg123/flac_format.c seems to be unchanged, and
can recognize neither the OggFlac 1.1.1 files newly created, nor
those from my pre-1.1.1 source which pointed out the problem.
(Is there a more appropriate list for ogg-utilities, say, ogg-dev@,
to handle kitchen-sink applications like ogg123?)

I've now patched flac_format.c in my copy of the ogg123 distribution
based on the 1.1.1 description you pointed me to, and as an after-
thought, added my hacks to recognize the unofficial mapping that I
used to see, as well as the original code.  (Was the original code
to find `fLaC' at position 28 ever correct?)

Here's my hack, in case someone who knows more about the history
of the FLAC->Ogg mapping wants to rewrite it to be correct and
apply it to vorbis-tools/ogg123/flac_format.c ...


--- flac_format.c-DIST	Sun Aug 22 20:33:25 2004
+++ flac_format.c	Fri Dec 17 10:41:57 2004
@@ -96,13 +96,35 @@
 
 int oggflac_can_decode (data_source_t *source)
 {
+#if 0  /* Wooo, this ain't right, at least not now... */
   char buf[32];
+#else  /* XXXX the below is based on observation of a real file 1.1.1 flac */
+  char buf[41];
+#endif
   int len;
 
+#if 0  /* Wooo, this ain't right, at least not now... */
   len = source->transport->peek(source, buf, sizeof(char), 32);
   
   if (len >= 32 && memcmp(buf, "OggS", 4) == 0
                 && memcmp(buf+28, "fLaC", 4) == 0)
+#else  /* XXXX the below is based on observation of a real file 1.1.1 flac */
+  len = source->transport->peek(source, buf, sizeof(char), 41);
+  
+/* single check    if (len >= 32 && memcmp(buf, "OggS", 4) == 0
+                && memcmp(buf+29, "FLAC", 4) == 0
+                && memcmp(buf+37, "fLaC", 4) == 0) XXX */
+  if ( (len >= 32 && memcmp(buf, "OggS", 4) == 0
+                && memcmp(buf+29, "FLAC", 4) == 0
+                && memcmp(buf+37, "fLaC", 4) == 0)
+/* XXX HACK compatibility below with the original code, plus flac < 1.1.1 */
+/* my previous hack below */
+  || (len >= 32 && memcmp(buf, "OggS", 4) == 0
+                && memcmp(buf+28, "FLAC", 4) == 0)
+/* XXX HACK original below; did this ever work? */
+  || (len >= 32 && memcmp(buf, "OggS", 4) == 0
+                && memcmp(buf+28, "fLaC", 4) == 0) )
+#endif
     return 1; /* Ogg FLAC */
   else
     return 0;


It still seems that seeking within the OggFLAC file within ogg123
does not work, and reading the source, probably doesn't work for
speex files either.

> > Out of curiosity, how easy should it be to implement
> > seeking within an OggFLAC file, as opposed to a plain
> > ordinary FLAC file?

> seeking was not supporting in libOggFLAC until FLAC 1.1.1

The deficiency is again in ogg123, flac_format.c , and probably
needs to be hacked based on, say, the ovf_seek function, somewhat
like I started, before I decided it needed more work than just
modifying ovf_seek as a template.


@@ -258,7 +280,27 @@
 
 int flac_seek (decoder_t *decoder, double offset, int whence)
 {
+#if 0 /* XXX Ugly HACK to see if we can get this working */
   return 0;  /* No seeking at this time */
+#else /* XXX */
+  flac_private_t *priv = decoder->private;
+  int ret;
+  double cur;
+#if 0 /* HACK stolen from ogg vorbis seek, but not ported to flac yet */
+  if (whence == DECODER_SEEK_CUR) {
+    cur = ov_time_tell(&priv->vf);
+    if (cur >= 0.0)
+      offset += cur;
+    else
+      return 0;
+  }
+  ret = ov_time_seek(&priv->vf, offset);
+  if (ret == 0)
+    return 1;
+  else
+#endif /* HACK stolen */
+    return 0;
+#endif
 }
 
 
Hey, I just woke up.  You expect me to come up with working code from
nothing?  Anyway, I may poke at it later, if my brain ever gets started.
Or anyone else is welcome to come up with something.


thanks
barry bouwsma