Add support for more than 1000 cue points in WAV and AIFF files (#434)

This work was originally submitted by Diemo Schwarz <schwarz@ircam.fr> in
PR https://github.com/erikd/libsndfile/pull/197 which was rebased and
cleaned up.

Closes: https://github.com/erikd/libsndfile/pull/197

src/aiff.c

@@ -794,8 +794,8 @@ aiff_read_header (SF_PRIVATE *psf, COMM_CHUNK *comm_fmt)
 						if (paiff->markstr == NULL)
 							return SFE_MALLOC_FAILED ;
 
-						if (mark_count > 1000)
+						if (mark_count > 2500) /* 2500 is close to the largest number of cues possible because of block sizes */
-						{	psf_log_printf (psf, "  More than 1000 markers, skipping!\n") ;
+						{	psf_log_printf (psf, "  More than 2500 markers, skipping!\n") ;
 							psf_binheader_readf (psf, "j", chunk_size - bytesread) ;
 							break ;
 						} ;

src/common.c

@@ -1,5 +1,5 @@
 /*
-** Copyright (C) 1999-2017 Erik de Castro Lopo <erikd@mega-nerd.com>
+** Copyright (C) 1999-2018 Erik de Castro Lopo <erikd@mega-nerd.com>
 **
 ** This program is free software; you can redistribute it and/or modify
 ** it under the terms of the GNU Lesser General Public License as published by
@@ -1243,8 +1243,12 @@ psf_memset (void *s, int c, sf_count_t len)
 
 typedef SF_CUES_VAR (0) SF_CUES_0 ;
 
+/* calculate size of SF_CUES struct given number of cues */
 #define SF_CUES_VAR_SIZE(count)	(sizeof (SF_CUES_0) + count * sizeof (SF_CUE_POINT))
 
+/* calculate number of cues in SF_CUES struct given data size */
+#define SF_CUES_COUNT(datasize) (((datasize) - sizeof (uint32_t)) / sizeof (SF_CUE_POINT))
+
 SF_CUES *
 psf_cues_alloc (uint32_t cue_count)
 {	SF_CUES *pcues = calloc (1, SF_CUES_VAR_SIZE (cue_count)) ;
@@ -1254,11 +1258,16 @@ psf_cues_alloc (uint32_t cue_count)
 } /* psf_cues_alloc */
 
 SF_CUES *
-psf_cues_dup (const void * ptr)
+psf_cues_dup (const void * ptr, size_t datasize)
 {	const SF_CUES *pcues = ptr ;
-	SF_CUES *pnew = psf_cues_alloc (pcues->cue_count) ;
+	SF_CUES *pnew = NULL ;
+
+	if (pcues->cue_count <= SF_CUES_COUNT (datasize))
+	{	/* check that passed-in datasize is consistent with cue_count in passed-in SF_CUES struct */
+		pnew = psf_cues_alloc (pcues->cue_count) ;
+		memcpy (pnew, pcues, SF_CUES_VAR_SIZE (pcues->cue_count)) ;
+	}
 
-	memcpy (pnew, pcues, SF_CUES_VAR_SIZE (pcues->cue_count)) ;
 	return pnew ;
 } /* psf_cues_dup */
 
@@ -1266,7 +1275,7 @@ void
 psf_get_cues (SF_PRIVATE * psf, void * data, size_t datasize)
 {
 	if (psf->cues)
-	{	uint32_t cue_count = (datasize - sizeof (uint32_t)) / sizeof (SF_CUE_POINT) ;
+	{	uint32_t cue_count = SF_CUES_COUNT (datasize) ;
 
 		cue_count = SF_MIN (cue_count, psf->cues->cue_count) ;
 		memcpy (data, psf->cues, SF_CUES_VAR_SIZE (cue_count)) ;

src/common.h

@@ -1002,7 +1002,7 @@ psf_strlcpy (char *dest, size_t n, const char *src)
 
 void	*psf_memset (void *s, int c, sf_count_t n) ;
 
-SF_CUES * psf_cues_dup (const void * ptr) ;
+SF_CUES * psf_cues_dup (const void * ptr, size_t datasize) ;
 SF_CUES * psf_cues_alloc (uint32_t cue_count) ;
 void psf_get_cues (SF_PRIVATE * psf, void * data, size_t datasize) ;
 

src/sndfile.c

@@ -1255,7 +1255,7 @@ sf_command	(SNDFILE *sndfile, int command, void *data, int datasize)
 			return SF_FALSE ;
 
 		case SFC_GET_CUE :
-			if (datasize != sizeof (SF_CUES) || data == NULL)
+			if (datasize < (int) sizeof (uint32_t) || data == NULL)
 			{	psf->error = SFE_BAD_COMMAND_PARAM ;
 				return SF_FALSE ;
 				} ;
@@ -1269,12 +1269,11 @@ sf_command	(SNDFILE *sndfile, int command, void *data, int datasize)
 			{	psf->error = SFE_CMD_HAS_DATA ;
 				return SF_FALSE ;
 				} ;
-			if (datasize != sizeof (SF_CUES) || data == NULL)
+			if (datasize < (int) sizeof (uint32_t) || data == NULL)
 			{	psf->error = SFE_BAD_COMMAND_PARAM ;
 				return SF_FALSE ;
 				} ;
-
+			if (psf->cues == NULL && (psf->cues = psf_cues_dup (data, datasize)) == NULL)
-			if (psf->cues == NULL && (psf->cues = psf_cues_dup (data)) == NULL)
 			{	psf->error = SFE_MALLOC_FAILED ;
 				return SF_FALSE ;
 				} ;

src/wav.c

@@ -473,9 +473,9 @@ wav_read_header	(SF_PRIVATE *psf, int *blockalign, int *framesperblock)
 						bytesread = psf_binheader_readf (psf, "4", &cue_count) ;
 						psf_log_printf (psf, "%M : %u\n", marker, chunk_size) ;
 
-						if (cue_count > 1000)
+						if (cue_count > 2500) /* 2500 is close to the largest number of cues possible because of block sizes */
 						{	psf_log_printf (psf, "  Count : %u (skipping)\n", cue_count) ;
-							psf_binheader_readf (psf, "j", (cue_count > 20 ? 20 : cue_count) * 24) ;
+							psf_binheader_readf (psf, "j", chunk_size - bytesread) ;
 							break ;
 							} ;
 
@@ -492,11 +492,15 @@ wav_read_header	(SF_PRIVATE *psf, int *blockalign, int *framesperblock)
 								break ;
 							bytesread += thisread ;
 
-							psf_log_printf (psf,	"   Cue ID : %2d"
+							if (cue_index < 10) /* avoid swamping log buffer with cues */
-													"  Pos : %5u  Chunk : %M"
+								psf_log_printf (psf,	"   Cue ID : %2d"
-													"  Chk Start : %d  Blk Start : %d"
+											"  Pos : %5u  Chunk : %M"
-													"  Offset : %5d\n",
+											"  Chk Start : %d  Blk Start : %d"
-									id, position, chunk_id, chunk_start, block_start, offset) ;
+											"  Offset : %5d\n",
+										id, position, chunk_id, chunk_start, block_start, offset) ;
+							else if (cue_index == 10)
+								psf_log_printf (psf,	"   (Skipping)\n") ;
+
 							psf->cues->cue_points [cue_index].indx = id ;
 							psf->cues->cue_points [cue_index].position = position ;
 							psf->cues->cue_points [cue_index].fcc_chunk = chunk_id ;

src/wavlike.c

@@ -1011,11 +1011,25 @@ wavlike_subchunk_parse (SF_PRIVATE *psf, int chunk, uint32_t chunk_length)
 
 						bytesread += psf_binheader_readf (psf, "b", buffer, chunk_size) ;
 						buffer [chunk_size] = 0 ;
-						psf_log_printf (psf, "    %M : %u : %s\n", chunk, mark_id, buffer) ;
+
+						if (mark_id < 10) /* avoid swamping log buffer with labels */
+							psf_log_printf (psf, "    %M : %u : %s\n", chunk, mark_id, buffer) ;
+						else if (mark_id == 10)
+							psf_log_printf (psf, "    (Skipping)\n") ;
+
+						if (psf->cues)
+ 						{	unsigned int i = 0 ;
+
+							/* find id to store label */
+							while (i < psf->cues->cue_count  &&  psf->cues->cue_points [i].indx != mark_id)
+								i++ ;
+
+							if (i < psf->cues->cue_count)
+								strncpy (psf->cues->cue_points [i].name, buffer, 256) ;
+							} ;
 						} ;
 					break ;
 
-
 			case DISP_MARKER :
 			case ltxt_MARKER :
 			case note_MARKER :

tests/command_test.c

@@ -1,5 +1,5 @@
 /*
-** Copyright (C) 2001-2017 Erik de Castro Lopo <erikd@mega-nerd.com>
+** Copyright (C) 2001-2018 Erik de Castro Lopo <erikd@mega-nerd.com>
 **
 ** This program is free software; you can redistribute it and/or modify
 ** it under the terms of the GNU General Public License as published by
@@ -48,7 +48,8 @@ static	void	format_tests			(void) ;
 static	void	calc_peak_test			(int filetype, const char *filename, int channels) ;
 static	void	truncate_test			(const char *filename, int filetype) ;
 static	void	instrument_test			(const char *filename, int filetype) ;
-static	void	cue_test			(const char *filename, int filetype) ;
+static	void	cue_test				(const char *filename, int filetype) ;
+static	void	cue_test_var			(const char *filename, int filetype, int count) ;
 static	void	channel_map_test		(const char *filename, int filetype) ;
 static	void	current_sf_info_test	(const char *filename) ;
 static	void	raw_needs_endswap_test	(const char *filename, int filetype) ;
@@ -59,7 +60,7 @@ static	void	broadcast_coding_history_test	(const char *filename) ;
 static	void	broadcast_coding_history_size	(const char *filename) ;
 
 /* Cart Chunk tests */
-static void	cart_test			(const char *filename, int filetype) ;
+static void	cart_test				(const char *filename, int filetype) ;
 static void	cart_rdwr_test			(const char *filename, int filetype) ;
 
 /* Force the start of this buffer to be double aligned. Sparc-solaris will
@@ -146,8 +147,18 @@ main (int argc, char *argv [])
 		} ;
 
 	if (do_all || strcmp (argv [1], "cue") == 0)
-	{	cue_test ("cue.wav", SF_FORMAT_WAV | SF_FORMAT_PCM_16) ;
+	{	/* 2500 is close to the largest number of cues possible because of block sizes (enforced in aiff.c, wav.c) */
+		int cuecounts [] = { -1, 0, 1, 10, 100, 101, 1000, 1001, 2500 } ;
+		unsigned int i ;
+
+		cue_test ("cue.wav", SF_FORMAT_WAV | SF_FORMAT_PCM_16) ;
 		cue_test ("cue.aiff" , SF_FORMAT_AIFF | SF_FORMAT_PCM_24) ;
+
+		for (i = 0 ; i < ARRAY_LEN (cuecounts) ; i++)
+		{	cue_test_var ("cue.wav", SF_FORMAT_WAV | SF_FORMAT_PCM_16, cuecounts [i]) ;
+			cue_test_var ("cue.aiff", SF_FORMAT_AIFF | SF_FORMAT_PCM_24, cuecounts [i]) ;
+			} ;
+
 		test_count ++ ;
 		} ;
 
@@ -835,6 +846,50 @@ instrument_test (const char *filename, int filetype)
 	puts ("ok") ;
 } /* instrument_test */
 
+static void
+print_cue (SF_CUES *cue, int i)
+{
+	printf ("   indx[%d]       : %d\n"
+		"   position      : %u\n"
+		"   fcc_chunk     : %x\n"
+		"   chunk_start   : %d\n"
+		"   block_start   : %d\n"
+		"   sample_offset : %u\n"
+		"   name          : %s\n",
+		i,
+		cue->cue_points [i].indx,
+		cue->cue_points [i].position,
+		cue->cue_points [i].fcc_chunk,
+		cue->cue_points [i].chunk_start,
+		cue->cue_points [i].block_start,
+		cue->cue_points [i].sample_offset,
+		cue->cue_points [i].name) ;
+}
+
+static int
+cue_compare (SF_CUES *write_cue, SF_CUES *read_cue, size_t cue_size, int line)
+{
+	if (memcmp (write_cue, read_cue, cue_size) != 0)
+	{
+		printf ("\n\nLine %d : cue comparison failed.\n\n", line) ;
+		printf ("W  Cue count     : %d\n", write_cue->cue_count) ;
+		if (write_cue->cue_count > 0)
+			print_cue (write_cue, 0) ;
+		if (write_cue->cue_count > 2)	/* print last if at least 2 */
+			print_cue (write_cue, write_cue->cue_count - 1) ;
+
+		printf ("R  Cue count     : %d\n", read_cue->cue_count) ;
+		if (read_cue->cue_count > 0)
+			print_cue (read_cue, 0) ;
+		if (read_cue->cue_count > 2)	/* print last if at least 2 */
+			print_cue (read_cue, read_cue->cue_count - 1) ;
+
+		return SF_FALSE ;
+		} ;
+
+	return SF_TRUE ;
+} /* cue_compare */
+
 static void
 cue_rw_test (const char *filename)
 {	SNDFILE *sndfile ;
@@ -924,71 +979,8 @@ cue_test (const char *filename, int filetype)
 	check_log_buffer_or_die (file, __LINE__) ;
 	sf_close (file) ;
 
-	if (memcmp (&write_cue, &read_cue, sizeof (write_cue)) != 0)
+	if (cue_compare (&write_cue, &read_cue, sizeof (write_cue), __LINE__) == SF_FALSE)
-	{	printf ("\n\nLine %d : cue comparison failed.\n\n", __LINE__) ;
-		printf ("W  Cue count      : %d\n"
-			"   indx          : %d\n"
-			"   position      : %u\n"
-			"   fcc_chunk     : %x\n"
-			"   chunk_start   : %d\n"
-			"   block_start   : %d\n"
-			"   sample_offset : %u\n"
-			"   name          : %s\n"
-			"   indx          : %d\n"
-			"   position      : %u\n"
-			"   fcc_chunk     : %x\n"
-			"   chunk_start   : %d\n"
-			"   block_start   : %d\n"
-			"   sample_offset : %u\n"
-			"   name           : %s\n",
-			write_cue.cue_count,
-			write_cue.cue_points [0].indx,
-			write_cue.cue_points [0].position,
-			write_cue.cue_points [0].fcc_chunk,
-			write_cue.cue_points [0].chunk_start,
-			write_cue.cue_points [0].block_start,
-			write_cue.cue_points [0].sample_offset,
-			write_cue.cue_points [0].name,
-			write_cue.cue_points [1].indx,
-			write_cue.cue_points [1].position,
-			write_cue.cue_points [1].fcc_chunk,
-			write_cue.cue_points [1].chunk_start,
-			write_cue.cue_points [1].block_start,
-			write_cue.cue_points [1].sample_offset,
-			write_cue.cue_points [1].name) ;
-		printf ("R  Cue count      : %d\n"
-			"   indx          : %d\n"
-			"   position      : %u\n"
-			"   fcc_chunk     : %x\n"
-			"   chunk_start   : %d\n"
-			"   block_start   : %d\n"
-			"   sample_offset : %u\n"
-			"   name          : %s\n"
-			"   indx          : %d\n"
-			"   position      : %u\n"
-			"   fcc_chunk     : %x\n"
-			"   chunk_start   : %d\n"
-			"   block_start   : %d\n"
-			"   sample_offset : %u\n"
-			"   name          : %s\n",
-			read_cue.cue_count,
-			read_cue.cue_points [0].indx,
-			read_cue.cue_points [0].position,
-			read_cue.cue_points [0].fcc_chunk,
-			read_cue.cue_points [0].chunk_start,
-			read_cue.cue_points [0].block_start,
-			read_cue.cue_points [0].sample_offset,
-			read_cue.cue_points [0].name,
-			read_cue.cue_points [1].indx,
-			read_cue.cue_points [1].position,
-			read_cue.cue_points [1].fcc_chunk,
-			read_cue.cue_points [1].chunk_start,
-			read_cue.cue_points [1].block_start,
-			read_cue.cue_points [1].sample_offset,
-			read_cue.cue_points [1].name) ;
-
 			exit (1) ;
-		} ;
 
 	if (0) cue_rw_test (filename) ;
 
@@ -996,6 +988,66 @@ cue_test (const char *filename, int filetype)
 	puts ("ok") ;
 } /* cue_test */
 
+/* calculate size of SF_CUES struct given number of cues */
+#define SF_CUES_SIZE(count)	(sizeof (uint32_t) + sizeof (SF_CUE_POINT) * (count))
+
+static void
+cue_test_var (const char *filename, int filetype, int count)
+{	size_t cues_size = SF_CUES_SIZE (count) ;
+	SF_CUES *write_cue = calloc (1, cues_size) ;
+	SF_CUES *read_cue = calloc (1, cues_size) ;
+	SNDFILE	*file ;
+	SF_INFO	sfinfo ;
+	char name [40] ;
+	int i ;
+
+	snprintf (name, sizeof (name), "cue_test_var %d", count) ;
+	print_test_name (name, filename) ;
+
+	if (write_cue == NULL || read_cue == NULL)
+	{	printf ("ok (can't alloc)\n") ;
+		return ;
+		} ;
+
+	write_cue->cue_count = count ;
+	for (i = 0 ; i < count ; i++)
+	{	write_cue->cue_points [i] = (SF_CUE_POINT) { i, 0, data_MARKER, 0, 0, i, "" } ;
+		if (filetype == (SF_FORMAT_AIFF | SF_FORMAT_PCM_24))
+			snprintf (write_cue->cue_points [i].name, sizeof (write_cue->cue_points [i].name), "Cue%03d", i) ;
+		} ;
+
+	sfinfo.samplerate	= 11025 ;
+	sfinfo.format		= filetype ;
+	sfinfo.channels		= 1 ;
+
+	file = test_open_file_or_die (filename, SFM_WRITE, &sfinfo, SF_TRUE, __LINE__) ;
+	if (sf_command (file, SFC_SET_CUE, write_cue, cues_size) == SF_FALSE)
+	{	printf ("\n\nLine %d : sf_command (SFC_SET_CUE) failed with %d cues, datasize %ld --> error: %s\n\n", __LINE__, count, cues_size, sf_strerror (file)) ;
+		exit (1) ;
+		} ;
+	test_write_double_or_die (file, 0, double_data, BUFFER_LEN, __LINE__) ;
+	sf_close (file) ;
+
+	memset (read_cue, 0, cues_size) ;
+
+	file = test_open_file_or_die (filename, SFM_READ, &sfinfo, SF_TRUE, __LINE__) ;
+
+	if (sf_command (file, SFC_GET_CUE, read_cue, cues_size) == SF_FALSE)
+	{	printf ("\n\nLine %d : sf_command (SFC_GET_CUE) failed with %d cues, datasize %ld --> error: %s\n\n", __LINE__, count, cues_size, sf_strerror (file)) ;
+		exit (1) ;
+		} ;
+	check_log_buffer_or_die (file, __LINE__) ;
+	sf_close (file) ;
+
+	if (cue_compare (write_cue, read_cue, cues_size, __LINE__) == SF_FALSE)
+	{	printf ("\n\nLine %d : cue_compare failed.\n\n", __LINE__) ;
+		exit (1) ;
+		} ;
+
+	unlink (filename) ;
+	puts ("ok") ;
+} /* cue_test_var */
+
 static	void
 current_sf_info_test	(const char *filename)
 {	SNDFILE *outfile, *infile ;

tests/cue_test.c

@@ -0,0 +1,135 @@
+
+#include "sfconfig.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <math.h>
+#include <sndfile.h>
+
+
+static void *
+get_cues (const char *filename, double *sr)
+{
+    SNDFILE    *file;
+    SF_INFO	sfinfo;
+
+    unsigned int err, size;
+    uint32_t count = 0;
+    SF_CUES_VAR(0) *info;
+
+    if ((file = sf_open(filename, SFM_READ, &sfinfo)) == NULL)
+    {
+	printf("can't open file '%s'\n", filename);
+	exit(1);
+    }
+
+    printf("\n---- get cues of file '%s'\n", filename);
+
+    if ((err = sf_command(file, SFC_GET_CUE_COUNT, &count, sizeof(uint32_t))) == SF_FALSE)
+    {
+	if (sf_error(file))
+	{
+	    printf("can't get cue info size for file '%s' (arg size %lu), err %s\n", 
+		   filename, sizeof(uint32_t), sf_strerror(file));
+	    exit(2);
+	}
+	else
+	    printf("no cue info for file '%s'\n", filename);
+	return NULL;
+    }
+	
+    size = sizeof(*info) + count * sizeof(SF_CUE_POINT);
+    printf("number of cues %d  size %d\n", count, size);
+
+    if (!(info = malloc(size)))
+	return NULL;
+
+    if (sf_command(file, SFC_GET_CUE, info, size) == SF_FALSE)
+    {
+	printf("can't get cue info of size %d for file '%s' error %s\n", 
+	       size, filename, sf_strerror(file));
+	exit(3);
+    }
+
+    *sr = sfinfo.samplerate;
+    sf_close(file);
+
+    return info;
+}
+
+
+static void
+test_cues (const char *filename)
+{
+    unsigned int i;
+    double sr;
+    SF_CUES_VAR(0) *info = get_cues(filename, &sr);
+
+    if (info == NULL)
+	exit(1);
+    
+    for (i = 0; i < info->cue_count; i++)
+    {
+	int    pos = info->cue_points[i].position;
+	double t   = (double) pos / sr;
+	double expected = i < 8  ?  (double) i / 3.  :  10. / 3.;
+	double error = (double) fabs(t - expected);
+
+	printf("cue %02d: markerID %02d  position %6d  offset %6d (time %.3f  expected %.3f  diff %f)  label '%s'\n",
+	       i, info->cue_points[i].indx, pos, info->cue_points[i].sample_offset, t, expected, error, info->cue_points[i].name);
+
+	if (error > 0.025)
+	    exit(4);
+    }
+
+    free(info);
+}
+
+static void
+print_cues (const char *filename)
+{
+    unsigned int i;
+    double sr;
+    SF_CUES_VAR(0) *info = get_cues(filename, &sr);
+
+    if (info == NULL)
+	exit(1);
+    
+    for (i = 0; i < info->cue_count; i++)
+    {
+	int    pos    = info->cue_points[i].position;
+	int    indx   = info->cue_points[i].indx;
+	int    cstart = info->cue_points[i].chunk_start;
+	int    bstart = info->cue_points[i].block_start;
+	int    offset = info->cue_points[i].sample_offset;
+	const char *name = info->cue_points[i].name;
+	double t   = (double) pos / sr;
+
+	if (cstart != 0  ||  bstart != 0)
+	    printf("cue %02d time %7.3f: markerID %02d  position %8d  chunk_start %d  block_start %d  offset %8d  label '%s'\n",
+		   i, t, indx, pos, offset, cstart, bstart, name);
+	else
+	    printf("cue %02d  time %7.3f: markerID %02d  position %8d  offset %8d  label '%s'\n",
+		   i, t, indx, pos, offset, name);
+    }
+
+    free(info);
+}
+
+
+int
+main (int argc, char **argv)
+{
+    int i;
+    
+    if (argc > 1)
+	for (i = 1; i < argc; i++)
+	    print_cues(argv[i]);
+    else
+    {
+	test_cues("clickpluck24.wav");
+	test_cues("clickpluck.wav");
+	test_cues("clickpluck.aiff");
+    }
+    return 0;
+}

tests/utils.tpl

@@ -1,6 +1,6 @@
 [+ AutoGen5 template h c +]
 /*
-** Copyright (C) 2002-2017 Erik de Castro Lopo <erikd@mega-nerd.com>
+** Copyright (C) 2002-2018 Erik de Castro Lopo <erikd@mega-nerd.com>
 **
 ** This program is free software; you can redistribute it and/or modify
 ** it under the terms of the GNU General Public License as published by
@@ -202,7 +202,7 @@ sf_count_t		file_length_fd (int fd) ;
 #define	M_PI		3.14159265358979323846264338
 #endif
 
-#define	LOG_BUFFER_SIZE		2048
+#define	LOG_BUFFER_SIZE		4096
 
 /*
 **	Neat solution to the Win32/OS2 binary file flage requirement.