Vorbis dev - Sep 2004 - Introducing ov_open_callbacksp and ov_clearp

Hello,

I've been looking to the libvorbisfile and got into troubles when trying to
use it: I need to compile it on a PalmOS and the libvorbisfile must be
compiled in ARM whereas calling code is in 68K. This implies that the
interface ov_open_callback is not usable because the OggVorbis_File *vf must
point to something in the target architecture (ARM) whereas the caller
cannot do that.

As well as for this I need a ov_info_basic function so that no internal
structure is returned (which seems to me a somewhat bad design because
you'll get stuck with that: each time an internal modification will occur,
outer code may have to be fixed or re-compiled).

I propose to add that to vorbisfile.h:
/* same as ov_clear but frees memory associated to vf too */
IMPORT_C int ov_clearp(OggVorbis_File *vf);
/* same ov_open_callbacks but also allocates the OggVorbis_File object into
*vf, if any error happen, returns same code as ov_open_callbacks and free
any memory allocated */
IMPORT_C int ov_open_callbacksp(void *datasource, OggVorbis_File **vf,
		char *initial, long ibytes, ov_callbacks callbacks);
IMPORT_C int ov_info_basic(OggVorbis_File *vf,int link, int *channels, long
*rate);

And that to vorbisfile.c:

EXPORT_C int ov_open_callbacksp(void *f,OggVorbis_File **vf,char
*initial,long ibytes,
    ov_callbacks callbacks){
  int ret;
  if (vf==NULL) return -1;
  *vf=_ogg_malloc(sizeof(OggVorbis_File));
  if (*vf==NULL) return -1;
  ret=_ov_open1(f,*vf,initial,ibytes,callbacks);
  if(ret!=0 || (ret=_ov_open2(*vf)) != 0)
  {
    ov_clearp(*vf);
    *vf = NULL;
  	return ret;
  }
  return 0;
}

EXPORT_C int ov_clearp(OggVorbis_File *vf){
	ov_clear(vf);
	if (vf)
		_ogg_free(vf);
	return 0;
}

EXPORT_C int ov_info_basic(OggVorbis_File *vf,int link, int *channels, long
*rate)
{
	vorbis_info *vi = ov_info(vf, link);
	if (vi==NULL)
		return -1;
	*channels = vi->channels;
	*rate = vi->rate;
	return 0;
}

Please tell if it interests anybody, if someone want to integrate it and has
problem feel free to ask any question (I do not know how I could had that to
the lib)

Best regards
Armel

On Tue, Sep 07, 2004 at 10:34:58AM +0200, Armel Asselin
wrote:
> Hello,
> 
> I've been looking to the libvorbisfile and got into troubles when
trying to
> use it: I need to compile it on a PalmOS and the libvorbisfile must be
> compiled in ARM whereas calling code is in 68K. This implies that the
> interface ov_open_callback is not usable because the OggVorbis_File *vf
must
> point to something in the target architecture (ARM) whereas the caller
> cannot do that.
If this is the case, not even the calling convention between the
codeblocks is standard C.  Embedded architectures generally == 'all
bets are off'.
> As well as for this I need a ov_info_basic function so that no internal
> structure is returned (which seems to me a somewhat bad design because
> you'll get stuck with that: each time an internal modification will
occur,
> outer code may have to be fixed or re-compiled).
This 'internal structure' as you describe it is fixed by the Vorbis
specification.  It won't change unless the specification changes, and
the specification today is fixed.
> I propose to add that to vorbisfile.h:
Your suggestion is not without some merit.  On the other hand, it very
nearly counts as a trivial change and is of limited usefulness to
others.  You'd be able to keep this snippet for your own code without
fear of a maintainability explosion.

(BTW, your 'p' suffix collides with an older convention where
'_p'
means 'this function is a query'.  Elsewhere in the Ogg code, the
convention is to call these functions 'create' and 'destroy'
rather
than 'init' and 'clear'.)

Monty

Conrad Parker tells:
> wouldn't it be simpler and more general to add constructors,
> destructors and access functions for OggVorbis_File and vorbis_info
> objects?
>From what Monty tells we could maybe call them: ov_create() and ov_destroy()
rather than ov_new/ov_delete

i.e.

EXPORT_C OggVorbis_File * ov_create (){
  return _ogg_malloc(sizeof(OggVorbis_File));
}

EXPORT_C int ov_destroy (OggVorbis_File *vf){
  if (vf) {
	ov_clear(vf);
	_ogg_free(vf);
  }
  return 0;
}

for accessors functions why not :) it's probably more general indeed

Armel

Hi Armel,

wouldn't it be simpler and more general to add constructors, destructors and
access functions for OggVorbis_File and vorbis_info objects?

eg.

EXPORT_C OggVorbis_File * ov_new (void){
  return _ogg_malloc(sizeof(OggVorbis_File));
}

EXPORT_C int ov_delete (OggVorbis_File *vf){
  if (vf) _ogg_free(vf);
  return 0;
}

int vorbis_info_get_channels (vorbis_info * vi)
{
  return vi->channels;
}

int vorbis_info_get_rate (vorbis_info * vi)
{
  return vi->rate;
}

etc....

Conrad.

On Tue, Sep 07, 2004 at 10:34:58AM +0200, Armel Asselin
wrote:
> Hello,
> 
> I've been looking to the libvorbisfile and got into troubles when
trying to
> use it: I need to compile it on a PalmOS and the libvorbisfile must be
> compiled in ARM whereas calling code is in 68K. This implies that the
> interface ov_open_callback is not usable because the OggVorbis_File *vf
must
> point to something in the target architecture (ARM) whereas the caller
> cannot do that.
> 
> As well as for this I need a ov_info_basic function so that no internal
> structure is returned (which seems to me a somewhat bad design because
> you'll get stuck with that: each time an internal modification will
occur,
> outer code may have to be fixed or re-compiled).
> 
> I propose to add that to vorbisfile.h:
> /* same as ov_clear but frees memory associated to vf too */
> IMPORT_C int ov_clearp(OggVorbis_File *vf);
> /* same ov_open_callbacks but also allocates the OggVorbis_File object into
> *vf, if any error happen, returns same code as ov_open_callbacks and free
> any memory allocated */
> IMPORT_C int ov_open_callbacksp(void *datasource, OggVorbis_File **vf,
> 		char *initial, long ibytes, ov_callbacks callbacks);
> IMPORT_C int ov_info_basic(OggVorbis_File *vf,int link, int *channels, long
> *rate);
> 
> And that to vorbisfile.c:
> 
> EXPORT_C int ov_open_callbacksp(void *f,OggVorbis_File **vf,char
> *initial,long ibytes,
>     ov_callbacks callbacks){
>   int ret;
>   if (vf==NULL) return -1;
>   *vf=_ogg_malloc(sizeof(OggVorbis_File));
>   if (*vf==NULL) return -1;
>   ret=_ov_open1(f,*vf,initial,ibytes,callbacks);
>   if(ret!=0 || (ret=_ov_open2(*vf)) != 0)
>   {
>     ov_clearp(*vf);
>     *vf = NULL;
>   	return ret;
>   }
>   return 0;
> }
> 
> EXPORT_C int ov_clearp(OggVorbis_File *vf){
> 	ov_clear(vf);
> 	if (vf)
> 		_ogg_free(vf);
> 	return 0;
> }
> 
> EXPORT_C int ov_info_basic(OggVorbis_File *vf,int link, int *channels, long
> *rate)
> {
> 	vorbis_info *vi = ov_info(vf, link);
> 	if (vi==NULL)
> 		return -1;
> 	*channels = vi->channels;
> 	*rate = vi->rate;
> 	return 0;
> }
> 
> Please tell if it interests anybody, if someone want to integrate it and
has
> problem feel free to ask any question (I do not know how I could had that
to
> the lib)
> 
> Best regards
> Armel
> 
> _______________________________________________
> Vorbis-dev mailing list
> Vorbis-dev@xiph.org
> http://lists.xiph.org/mailman/listinfo/vorbis-dev