From: Maximilian S. <max...@bu...> - 2007-11-25 16:00:07
Attachments:
2007_11_25_id3_genre.patch
|
Hi, attached a patch updating ID3 genres. I got the additional genres from the source code of easytag. Cheers, m. |
From: Philipp M. H. <pm...@ti...> - 2007-11-26 07:30:58
|
Hello! What about the following conversion, which reduces the maintainance overhead a little bit and removes a common programming error: On Sun, Nov 25, 2007 at 04:59:41PM +0100, Maximilian Schwerin wrote: > diff -r df03dd970f5f src/demuxers/id3.c > --- a/src/demuxers/id3.c Sat Nov 24 20:41:43 2007 +0100 > +++ b/src/demuxers/id3.c Sun Nov 25 16:52:07 2007 +0100 > @@ -45,7 +45,7 @@ > #include "bswap.h" > #include "id3.h" > > -#define ID3_GENRE_COUNT 126 > +#define ID3_GENRE_COUNT 148 -#define ID3_GENRE_COUNT 148 > static const char* const id3_genre[] = > {"Blues", "Classic Rock", "Country", "Dance", "Disco", > "Funk", "Grunge", "Hip-Hop", "Jazz", "Metal", > @@ -72,7 +72,11 @@ static const char* const id3_genre[] = > "Satire", "Slow Jam", "Club", "Tango", "Samba", > "Folklore", "Ballad", "Power Ballad", "Rhythmic Soul", "Freestyle", > "Duet", "Punk Rock", "Drum Solo", "A capella", "Euro-House", > - "Dance Hall" }; > + "Dance Hall", "Goa", "Drum & Bass", "Club-House", "Hardcore", "Terror", > + "Indie", "BritPop", "Negerpunk", "Polsk Punk", "Beat", > + "Christian Gangsta Rap", "Heavy Metal", "Black Metal", "Crossover", > + "Contemporary Christian", "Christian Rock", "Merengue", "Salsa", > + "Thrash Metal", "Anime", "JPop", "Synthpop" }; +#ifndef ARRAY_SIZE +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) +#endif +#define ID3_GENRE_COUNT ARRAY_SIZE(id3_genre) What abount moving ARRAY_SIZE to xineutils.h or xine_internals.h and also converting those 41 files of xine-lib, where you find several occurances of "sizeof(foo)/sizeof(*foo)" or "sizeof(foo)/sizeof(foo[0])" ./src/audio_out/audio_alsa_out.c ./src/audio_out/audio_fusionsound_out.c ./src/demuxers/demux_aiff.c ./src/demuxers/demux_avi.c ./src/demuxers/demux_dts.c ./src/input/input_v4l.c ./src/input/libdvdnav/vmcmd.c ./src/input/libreal/real.c ./src/post/goom/goomsl_yacc.c ./src/post/goom/plugin_info.c ./src/video_out/libdha/pci_db2c.awk ./src/video_out/video_out_fb.c ./src/video_out/video_out_opengl.c ./src/video_out/video_out_pgx32.c ./src/video_out/video_out_xcbshm.c ./src/video_out/video_out_xshm.c ./src/video_out/video_out_xxmc.c ./src/video_out/vidix/drivers/cyberblade_vid.c ./src/video_out/vidix/drivers/mach64_vid.c ./src/video_out/vidix/drivers/nvidia_vid.c ./src/video_out/vidix/drivers/pm2_vid.c ./src/video_out/vidix/drivers/pm3_vid.c ./src/video_out/vidix/drivers/radeon_vid.c ./src/video_out/vidix/drivers/savage_vid.c ./src/video_out/vidix/drivers/sis_vid.c ./src/video_out/vidix/drivers/unichrome_vid.c ./src/video_out/yuv2rgb.c 3rd party files: ./src/libdts/parse.c ./src/libffmpeg/ff_mpeg_parser.c ./src/libffmpeg/ff_audio_decoder.c ./src/libffmpeg/libavcodec/alpha/asm.h ./src/libffmpeg/libavcodec/dvdata.h ./src/libffmpeg/libavcodec/h263.c ./src/libffmpeg/libavcodec/h264.c ./src/libffmpeg/libavcodec/imgresample.c ./src/libffmpeg/ff_video_decoder.c ./src/libmpeg2new/libmpeg2/header.c ./src/libmusepack/streaminfo.c ./src/libreal/real_common.c ./src/libw32dll/DirectShow/iunk.h ./src/libw32dll/wine/win32.c BYtE Philipp -- / / (_)__ __ ____ __ Philipp Hahn / /__/ / _ \/ // /\ \/ / /____/_/_//_/\_,_/ /_/\_\ pm...@ti... |
From: Maximilian S. <max...@bu...> - 2007-11-26 08:02:53
|
Hi, > what about the following conversion, which reduces the maintainance > overhead a little bit and removes a common programming error: -- snipsnap -- Sounds like a good idea though I would suggest commiting in small chunks. Don't mix changes in functionality and refactoring as this makes testing harder. Cheers, m. |
From: Philipp M. H. <pm...@ti...> - 2007-11-30 09:24:51
|
Hello! On Mon, Nov 26, 2007 at 09:02:56AM +0100, Maximilian Schwerin wrote: > > what about the following conversion, which reduces the maintainance > > overhead a little bit and removes a common programming error: > > -- snipsnap -- > > Sounds like a good idea though I would suggest commiting in small chunks. Don't > mix changes in functionality and refactoring as this makes testing harder. Here is my proposed change: - replace sizeof(...)/sizeof(...[0]) constructs with ARRAY_SIZE() macro - replace constants describing the size of an array in loops for better readability (ID3_GENRE_COUNT, NUM_RESOLUTIONS, NUM_ACCEL_PRIORITY) - trailing whitespace cleanups in lines ±3 above and below those lines I seems to compile, but since my development machine is very old and has a heat problem, I didn't yet commit this change. Feedback would be welcomed. audio_out/audio_alsa_out.c | 8 ++++---- audio_out/audio_fusionsound_out.c | 4 ++-- demuxers/demux_aiff.c | 4 ++-- demuxers/demux_dts.c | 2 +- demuxers/id3.c | 11 +++++------ input/input_v4l.c | 7 +++---- input/libdvdnav/vmcmd.c | 3 ++- video_out/video_out_fb.c | 14 +++++--------- video_out/video_out_opengl.c | 8 ++++---- video_out/video_out_pgx32.c | 2 +- video_out/video_out_xcbshm.c | 4 ++-- video_out/video_out_xshm.c | 4 ++-- video_out/video_out_xxmc.c | 5 ++--- video_out/yuv2rgb.c | 3 ++- xine-engine/xine_internal.h | 2 ++ 15 files changed, 39 insertions(+), 42 deletions(-) diff -r 3d88fc984e4b src/audio_out/audio_alsa_out.c --- a/src/audio_out/audio_alsa_out.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/audio_out/audio_alsa_out.c Fri Nov 30 10:12:18 2007 +0100 @@ -132,14 +132,14 @@ static int my_snd_mixer_wait(snd_mixer_t struct pollfd *pfds = spfds; int err, count; - count = snd_mixer_poll_descriptors(mixer, pfds, sizeof(spfds) / sizeof(spfds[0])); + count = snd_mixer_poll_descriptors(mixer, pfds, ARRAY_SIZE(spfds)); if (count < 0) return count; - - if ((unsigned int) count > sizeof(spfds) / sizeof(spfds[0])) { + + if ((unsigned int) count > ARRAY_SIZE(spfds)) { pfds = malloc(count * sizeof(*pfds)); - + if (!pfds) return -ENOMEM; diff -r 3d88fc984e4b src/audio_out/audio_fusionsound_out.c --- a/src/audio_out/audio_fusionsound_out.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/audio_out/audio_fusionsound_out.c Fri Nov 30 10:12:18 2007 +0100 @@ -384,12 +384,12 @@ static int ao_fusionsound_control(ao_dri } -static ao_driver_t* open_plugin(audio_driver_class_t *ao_class, +static ao_driver_t* open_plugin(audio_driver_class_t *ao_class, const void *data ) { fusionsound_class_t *class = (fusionsound_class_t *) ao_class; fusionsound_driver_t *this; const char *args[] = { "xine", "--dfb:no-sighandler", "--fs:no-banner" }; - int argn = sizeof(args) / sizeof(args[0]); + int argn = ARRAY_SIZE(args); char **argp = (char **) args; DFBResult ret; diff -r 3d88fc984e4b src/demuxers/demux_aiff.c --- a/src/demuxers/demux_aiff.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/demuxers/demux_aiff.c Fri Nov 30 10:12:18 2007 +0100 @@ -117,8 +117,8 @@ static int open_aiff_file(demux_aiff_t * } chunk_type = _X_BE_32(&preamble[0]); chunk_size = _X_BE_32(&preamble[4]); - - if (chunk_size > sizeof(buffer) / sizeof(buffer[0])) { + + if (chunk_size > ARRAY_SIZE(buffer)) { /* the chunk is too large to fit in the buffer -> this cannot be an aiff chunk */ this->status = DEMUX_FINISHED; return 0; diff -r 3d88fc984e4b src/demuxers/demux_dts.c --- a/src/demuxers/demux_dts.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/demuxers/demux_dts.c Fri Nov 30 10:12:18 2007 +0100 @@ -164,7 +164,7 @@ static int open_dts_file(demux_dts_t *th ((peak[this->data_start+9] & 0x3c) >> 2)) + 1; sfreq = peak[this->data_start+8] & 0x0f; - if ((sfreq > sizeof(dts_sample_rates)/sizeof(int)) || + if ((sfreq > ARRAY_SIZE(dts_sample_rates)) || (dts_sample_rates[sfreq] == 0)) return 0; diff -r 3d88fc984e4b src/demuxers/id3.c --- a/src/demuxers/id3.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/demuxers/id3.c Fri Nov 30 10:12:18 2007 +0100 @@ -45,7 +45,6 @@ #include "bswap.h" #include "id3.h" -#define ID3_GENRE_COUNT (sizeof (id3_genre) / sizeof (id3_genre[0])) static const char* const id3_genre[] = {"Blues", "Classic Rock", "Country", "Dance", "Disco", "Funk", "Grunge", "Hip-Hop", "Jazz", "Metal", @@ -110,10 +109,10 @@ int id3v1_parse_tag (input_plugin_t *inp _x_meta_info_set(stream, XINE_META_INFO_TRACK_NUMBER, track); } - if (tag.genre < ID3_GENRE_COUNT) { + if (tag.genre < ARRAY_SIZE(id3_genre)) { _x_meta_info_set(stream, XINE_META_INFO_GENRE, id3_genre[tag.genre]); } - + } return 1; } else { @@ -160,7 +159,7 @@ static int id3v2_parse_genre(char* dest, index = 10 * index + (*src - '0'); src++; } else if (*src == ')') { - if (index < ID3_GENRE_COUNT) { + if (index < ARRAY_SIZE(id3_genre)) { strncpy(buf, id3_genre[index], len - (buf - dest)); buf += strlen(id3_genre[index]); } else { @@ -600,10 +599,10 @@ int id3v23_parse_tag(input_plugin_t *inp /* id3v2 "genre" parsing code. what a ugly format ! */ static int id3v24_parse_genre(char* dest, char *src, int len) { unsigned int index = 0; - + dest[0] = '\0'; if (sscanf(src, "%u", &index) == 1) { - if (index < ID3_GENRE_COUNT) { + if (index < ARRAY_SIZE(id3_genre)) { strncpy(dest, id3_genre[index], len); dest[len - 1] = '\0'; } else { diff -r 3d88fc984e4b src/input/input_v4l.c --- a/src/input/input_v4l.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/input/input_v4l.c Fri Nov 30 10:12:18 2007 +0100 @@ -90,7 +90,6 @@ static const resolution_t resolutions[] { 160, 120 } }; -#define NUM_RESOLUTIONS (sizeof(resolutions)/sizeof(resolutions[0])) #define RADIO_DEV "/dev/v4l/radio0" #define VIDEO_DEV "/dev/v4l/video0" @@ -817,11 +816,11 @@ static int open_video_capture_device(v4l entry->str_value, strerror(errno)); return 0; } - + lprintf("Device opened, tv %d\n", this->video_fd); - + /* figure out the resolution */ - for (j = 0; j < NUM_RESOLUTIONS; j++) + for (j = 0; j < ARRAY_SIZE(resolutions); j++) { if (resolutions[j].width <= this->video_cap.maxwidth && resolutions[j].height <= this->video_cap.maxheight diff -r 3d88fc984e4b src/input/libdvdnav/vmcmd.c --- a/src/input/libdvdnav/vmcmd.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/input/libdvdnav/vmcmd.c Fri Nov 30 10:12:19 2007 +0100 @@ -29,6 +29,7 @@ #include <inttypes.h> #include "dvdnav_internal.h" +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) /* freebsd compatibility */ #ifndef PRIu8 @@ -272,7 +273,7 @@ static void print_linksub_instruction(co uint32_t linkop = vm_getbits(command, 7, 8); uint32_t button = vm_getbits(command, 15, 6); - if(linkop < sizeof(link_table)/sizeof(char *) && link_table[linkop] != NULL) + if(linkop < ARRAY_SIZE(link_table) && link_table[linkop] != NULL) fprintf(MSG_OUT, "%s (button %" PRIu8 ")", link_table[linkop], button); else fprintf(MSG_OUT, "WARNING: Unknown linksub instruction (%i)", linkop); diff -r 3d88fc984e4b src/video_out/video_out_fb.c --- a/src/video_out/video_out_fb.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/video_out/video_out_fb.c Fri Nov 30 10:12:19 2007 +0100 @@ -442,12 +442,10 @@ static void fb_overlay_clut_yuv2rgb(fb_d { int i; clut_t* clut = (clut_t*)overlay->color; - + if(!overlay->rgb_clut) { - for(i = 0; - i < sizeof(overlay->color)/sizeof(overlay->color[0]); - i++) + for(i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb-> @@ -458,14 +456,12 @@ static void fb_overlay_clut_yuv2rgb(fb_d } overlay->rgb_clut++; } - + if(!overlay->hili_rgb_clut) { clut = (clut_t*) overlay->hili_color; - - for(i = 0; - i < sizeof(overlay->color)/sizeof(overlay->color[0]); - i++) + + for(i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb-> diff -r 3d88fc984e4b src/video_out/video_out_opengl.c --- a/src/video_out/video_out_opengl.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/video_out/video_out_opengl.c Fri Nov 30 10:12:19 2007 +0100 @@ -1416,7 +1416,7 @@ static void opengl_overlay_clut_yuv2rgb( clut_t* clut = (clut_t*) overlay->color; if (!overlay->rgb_clut) { - for (i = 0; i < sizeof(overlay->color)/sizeof(overlay->color[0]); i++) { + for (i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb->yuv2rgb_single_pixel_fun (frame->yuv2rgb, clut[i].y, clut[i].cb, clut[i].cr); @@ -1425,7 +1425,7 @@ static void opengl_overlay_clut_yuv2rgb( } if (!overlay->hili_rgb_clut) { clut = (clut_t*) overlay->hili_color; - for (i = 0; i < sizeof(overlay->color)/sizeof(overlay->color[0]); i++) { + for (i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb->yuv2rgb_single_pixel_fun(frame->yuv2rgb, clut[i].y, clut[i].cb, clut[i].cr); @@ -1876,9 +1876,9 @@ static vo_driver_t *opengl_open_plugin ( this->drawable, X11OSD_SHAPED); XUnlockDisplay (this->display); - render_fun_names = xine_xmalloc ((sizeof(opengl_rb)/sizeof(opengl_render_t)+1) + render_fun_names = xine_xmalloc ((ARRAY_SIZE(opengl_rb)+1) * sizeof (const char *)); - for (i = 0; i < sizeof (opengl_rb) / sizeof (opengl_render_t); i++) + for (i = 0; i < ARRAY_SIZE(opengl_rb); i++) render_fun_names[i] = opengl_rb[i].name; render_fun_names[i] = NULL; this->render_fun_id = config->register_enum (config, "video.output.opengl_renderer", diff -r 3d88fc984e4b src/video_out/video_out_pgx32.c --- a/src/video_out/video_out_pgx32.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/video_out/video_out_pgx32.c Fri Nov 30 10:12:19 2007 +0100 @@ -420,7 +420,7 @@ static void pgx32_update_frame_format(vo frame->format = format; frame->pitch = 2048; - for (i=0; i<sizeof(pitch_code_table)/sizeof(pitch_code_table[0]); i++) { + for (i = 0; i < ARRAY_SIZE(pitch_code_table); i++) { if ((pitch_code_table[i][0] >= frame->width) && (pitch_code_table[i][0] <= frame->pitch)) { frame->pitch = pitch_code_table[i][0]; frame->pitch_code = pitch_code_table[i][1]; diff -r 3d88fc984e4b src/video_out/video_out_xcbshm.c --- a/src/video_out/video_out_xcbshm.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/video_out/video_out_xcbshm.c Fri Nov 30 10:12:19 2007 +0100 @@ -512,7 +512,7 @@ static void xshm_overlay_clut_yuv2rgb(xs clut_t* clut = (clut_t*) overlay->color; if (!overlay->rgb_clut) { - for (i = 0; i < sizeof(overlay->color)/sizeof(overlay->color[0]); i++) { + for (i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb->yuv2rgb_single_pixel_fun (frame->yuv2rgb, clut[i].y, clut[i].cb, clut[i].cr); @@ -521,7 +521,7 @@ static void xshm_overlay_clut_yuv2rgb(xs } if (!overlay->hili_rgb_clut) { clut = (clut_t*) overlay->hili_color; - for (i = 0; i < sizeof(overlay->color)/sizeof(overlay->color[0]); i++) { + for (i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb->yuv2rgb_single_pixel_fun(frame->yuv2rgb, clut[i].y, clut[i].cb, clut[i].cr); diff -r 3d88fc984e4b src/video_out/video_out_xshm.c --- a/src/video_out/video_out_xshm.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/video_out/video_out_xshm.c Fri Nov 30 10:12:19 2007 +0100 @@ -607,7 +607,7 @@ static void xshm_overlay_clut_yuv2rgb(xs clut_t* clut = (clut_t*) overlay->color; if (!overlay->rgb_clut) { - for (i = 0; i < sizeof(overlay->color)/sizeof(overlay->color[0]); i++) { + for (i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb->yuv2rgb_single_pixel_fun (frame->yuv2rgb, clut[i].y, clut[i].cb, clut[i].cr); @@ -616,7 +616,7 @@ static void xshm_overlay_clut_yuv2rgb(xs } if (!overlay->hili_rgb_clut) { clut = (clut_t*) overlay->hili_color; - for (i = 0; i < sizeof(overlay->color)/sizeof(overlay->color[0]); i++) { + for (i = 0; i < ARRAY_SIZE(overlay->color); i++) { *((uint32_t *)&clut[i]) = frame->yuv2rgb->yuv2rgb_single_pixel_fun(frame->yuv2rgb, clut[i].y, clut[i].cb, clut[i].cr); diff -r 3d88fc984e4b src/video_out/video_out_xxmc.c --- a/src/video_out/video_out_xxmc.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/video_out/video_out_xxmc.c Fri Nov 30 10:12:19 2007 +0100 @@ -57,7 +57,6 @@ static const unsigned int accel_priority #endif XINE_XVMC_ACCEL_IDCT, XINE_XVMC_ACCEL_MOCOMP}; -#define NUM_ACCEL_PRIORITY (sizeof(accel_priority)/sizeof(accel_priority[0])) #ifndef XVMC_VLD #define XVMC_VLD 0 @@ -797,7 +796,7 @@ static int xxmc_find_context(xxmc_driver found = 0; curCap = NULL; - for (k = 0; k < NUM_ACCEL_PRIORITY; ++k) { + for (k = 0; k < ARRAY_SIZE(accel_priority); ++k) { request_accel_flags = xxmc->acceleration & accel_priority[k]; if (!request_accel_flags) continue; @@ -1197,7 +1196,7 @@ static int xxmc_accel_update(xxmc_driver * Test for possible use of a higher acceleration level. */ - for (k = 0; k < NUM_ACCEL_PRIORITY; ++k) { + for (k = 0; k < ARRAY_SIZE(accel_priority); ++k) { if (last_request & accel_priority[k]) return 0; if (new_request & accel_priority[k]) return 1; } diff -r 3d88fc984e4b src/video_out/yuv2rgb.c --- a/src/video_out/yuv2rgb.c Sun Nov 25 23:46:01 2007 +0100 +++ b/src/video_out/yuv2rgb.c Fri Nov 30 10:12:19 2007 +0100 @@ -40,6 +40,7 @@ */ #include "xineutils.h" +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) static int prof_scale_line = -1; @@ -1280,7 +1281,7 @@ static scale_line_func_t find_scale_line static int reported_for_step; #endif - for (i = 0; i < sizeof(scale_line)/sizeof(scale_line[0]); i++) { + for (i = 0; i < ARRAY_SIZE(scale_line); i++) { if (step == scale_line[i].src_step*32768/scale_line[i].dest_step) { #ifdef LOG if (step != reported_for_step) diff -r 3d88fc984e4b src/xine-engine/xine_internal.h --- a/src/xine-engine/xine_internal.h Sun Nov 25 23:46:01 2007 +0100 +++ b/src/xine-engine/xine_internal.h Fri Nov 30 10:12:19 2007 +0100 @@ -69,6 +69,8 @@ extern "C" { # include <xine/alphablend.h> #endif + +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) #define XINE_MAX_EVENT_LISTENERS 50 #define XINE_MAX_EVENT_TYPES 100 BYtE Philipp -- / / (_)__ __ ____ __ Philipp Hahn / /__/ / _ \/ // /\ \/ / /____/_/_//_/\_,_/ /_/\_\ pm...@ti... |
From: Darren S. <li...@yo...> - 2007-11-26 13:54:02
|
I demand that Philipp Matthias Hahn may or may not have written... > What about the following conversion, which reduces the maintainance > overhead a little bit and removes a common programming error: > On Sun, Nov 25, 2007 at 04:59:41PM +0100, Maximilian Schwerin wrote: > > diff -r df03dd970f5f src/demuxers/id3.c > > --- a/src/demuxers/id3.c Sat Nov 24 20:41:43 2007 +0100 > > +++ b/src/demuxers/id3.c Sun Nov 25 16:52:07 2007 +0100 [snip] > +#ifndef ARRAY_SIZE > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > +#endif > +#define ID3_GENRE_COUNT ARRAY_SIZE(id3_genre) ID3_GENRE_COUNT is defined using sizeof() - I didn't like the use of a non-calculated constant where it can be calculated at compile time. > What abount moving ARRAY_SIZE to xineutils.h or xine_internals.h and also > converting those 41 files of xine-lib, where you find several occurances of > "sizeof(foo)/sizeof(*foo)" or "sizeof(foo)/sizeof(foo[0])" [snip lists] I see no problem with that, but I'd be inclined to leave the "third-party" files alone. If you want to do this, now would be a good time... :-) -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICIENT. You cannot see farther than others by standing on the feet of giants. |
From: Philipp M. H. <pm...@ti...> - 2007-11-26 14:22:20
|
Hello! On Mon, Nov 26, 2007 at 01:38:41PM +0000, Darren Salt wrote: > I demand that Philipp Matthias Hahn may or may not have written... ... > > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > > +#define ID3_GENRE_COUNT ARRAY_SIZE(id3_genre) > > ID3_GENRE_COUNT is defined using sizeof() - I didn't like the use of a > non-calculated constant where it can be calculated at compile time. Me too, that is why I propose this change. > I see no problem with that, but I'd be inclined to leave the "third-party" > files alone. If you want to do this, now would be a good time... :-) Since I'm no hg/Mercurial guru, do I have to change anything regarding my current hg setup (e.g. changeing to another branch)? $ head .hg/branch .hg/hgrc ==> .hg/branch <== default ==> .hg/hgrc <== [paths] default = ssh://pm...@hg...//hg/xine-lib/xine-lib BYtE Philipp -- / / (_)__ __ ____ __ Philipp Hahn / /__/ / _ \/ // /\ \/ / /____/_/_//_/\_,_/ /_/\_\ pm...@ti... |
From: Darren S. <li...@yo...> - 2007-11-26 14:52:40
|
I demand that Philipp Matthias Hahn may or may not have written... > On Mon, Nov 26, 2007 at 01:38:41PM +0000, Darren Salt wrote: >> I demand that Philipp Matthias Hahn may or may not have written... > ... >>> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) >>> +#define ID3_GENRE_COUNT ARRAY_SIZE(id3_genre) >> ID3_GENRE_COUNT is defined using sizeof() - I didn't like the use of a >> non-calculated constant where it can be calculated at compile time. > Me too, that is why I propose this change. >> I see no problem with that, but I'd be inclined to leave the "third-party" >> files alone. If you want to do this, now would be a good time... :-) > Since I'm no hg/Mercurial guru, do I have to change anything regarding my > current hg setup (e.g. changing to another branch)? > $ head .hg/branch .hg/hgrc > ==> .hg/branch <== > default Repository-internal branches aren't supported by 0.9.1 (the version in use on alioth), so we're not using them. > ==> .hg/hgrc <== > [paths] > default = ssh://pm...@hg...//hg/xine-lib/xine-lib That looks fine. (Hmm, mutt ignores Mail-Followup-To... :-\ ) -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICIENT. "Sheesh! It's not rocket philosophy!" |
From: <fla...@gm...> - 2007-11-26 14:23:24
|
Philipp Matthias Hahn <pm...@ti...> writes: > ./src/video_out/vidix/drivers/cyberblade_vid.c > ./src/video_out/vidix/drivers/mach64_vid.c > ./src/video_out/vidix/drivers/nvidia_vid.c > ./src/video_out/vidix/drivers/pm2_vid.c > ./src/video_out/vidix/drivers/pm3_vid.c > ./src/video_out/vidix/drivers/radeon_vid.c > ./src/video_out/vidix/drivers/savage_vid.c > ./src/video_out/vidix/drivers/sis_vid.c > ./src/video_out/vidix/drivers/unichrome_vid.c These are 3rd party too. -- Diego "Flameeyes" Pettenò http://farragut.flameeyes.is-a-geek.org/ |
From: <fla...@gm...> - 2007-11-26 14:29:19
|
Philipp Matthias Hahn <pm...@ti...> writes: > Me too, that is why I propose this change. I think Darren meant that sizeof(foo)/sizeof(foo[0]) would be replaced in all parts of the code, and thus requiring calculation of it at runtime. This shouldn't really happen, I think, as GCC should be smart enough. On the other hand if we use that constant many times it might be worth decaring it as static const int ID3_GENRE_COUNT = sizeof(id3_genre)/sizeof(id3_genre[0]); Anyway, my vote is for having the define in xine_internal.h, we shouldn't really make xine a generic purposes utils library ;) -- Diego "Flameeyes" Pettenò http://farragut.flameeyes.is-a-geek.org/ |
From: Darren S. <li...@yo...> - 2007-11-26 14:59:30
|
I demand that Diego 'Flameeyes' Petten=C3=B2 may or may not have written.= .. > Philipp Matthias Hahn <pm...@ti...> writes: >> Me too, that is why I propose this change. > I think Darren meant that sizeof(foo)/sizeof(foo[0]) would be replaced = in > all parts of the code, and thus requiring calculation of it at runtime.= I don't. :-) [snip] > On the other hand if we use that constant many times it might be worth > decaring it as [a static const variable]. I don't think that that's worthwhile in this case. [snip] --=20 | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICI= ENT. Support your local church. Worship at the Bank of England. |
From: <fla...@gm...> - 2007-11-30 12:02:41
|
Philipp Matthias Hahn <pm...@ti...> writes: > diff -r 3d88fc984e4b src/input/libdvdnav/vmcmd.c > --- a/src/input/libdvdnav/vmcmd.c Sun Nov 25 23:46:01 2007 +0100 > +++ b/src/input/libdvdnav/vmcmd.c Fri Nov 30 10:12:19 2007 +0100 Stop here. Do NOT touch libdvdnav unlessto fix bugs, it's contributed code (and outdated fwiw). =2D-=20 Diego "Flameeyes" Petten=C3=B2 http://farragut.flameeyes.is-a-geek.org/ |
From: <fla...@gm...> - 2007-11-30 12:05:03
|
Philipp Matthias Hahn <pm...@ti...> writes: > diff -r 3d88fc984e4b src/video_out/yuv2rgb.c > --- a/src/video_out/yuv2rgb.c Sun Nov 25 23:46:01 2007 +0100 > +++ b/src/video_out/yuv2rgb.c Fri Nov 30 10:12:19 2007 +0100 > @@ -40,6 +40,7 @@ > */ >=20=20 > #include "xineutils.h" > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) >=20=20 > static int prof_scale_line =3D -1; >=20=20 Why the double define? I'm not really a fine fo those, can't it just include xine_internal.h? =2D-=20 Diego "Flameeyes" Petten=C3=B2 http://farragut.flameeyes.is-a-geek.org/ |
From: Philipp M. H. <pm...@ti...> - 2007-12-03 09:55:11
|
Hello! On Fri, Nov 30, 2007 at 01:02:10PM +0100, Diego 'Flameeyes' Pettenò wrote: > Philipp Matthias Hahn <pm...@ti...> writes: > > > diff -r 3d88fc984e4b src/input/libdvdnav/vmcmd.c > > --- a/src/input/libdvdnav/vmcmd.c Sun Nov 25 23:46:01 2007 +0100 > > +++ b/src/input/libdvdnav/vmcmd.c Fri Nov 30 10:12:19 2007 +0100 > > Stop here. Do NOT touch libdvdnav unlessto fix bugs, it's contributed > code (and outdated fwiw). Okay, I will drop that change. On Fri, Nov 30, 2007 at 01:03:46PM +0100, Diego 'Flameeyes' Pettenò wrote: > Philipp Matthias Hahn <pm...@ti...> writes: > > > diff -r 3d88fc984e4b src/video_out/yuv2rgb.c > > --- a/src/video_out/yuv2rgb.c Sun Nov 25 23:46:01 2007 +0100 > > +++ b/src/video_out/yuv2rgb.c Fri Nov 30 10:12:19 2007 +0100 > > @@ -40,6 +40,7 @@ > > */ > > > > #include "xineutils.h" > > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > > > > static int prof_scale_line = -1; > > > > Why the double define? I'm not really a fine fo those, can't it just > include xine_internal.h? Yes, that would have been the alternative. If it is preferred to include xine_internal.h, I'll change that too. BYtE Philipp -- / / (_)__ __ ____ __ Philipp Hahn / /__/ / _ \/ // /\ \/ / /____/_/_//_/\_,_/ /_/\_\ pm...@ti... |