From: Andre Noll Date: Sat, 13 Aug 2011 10:17:51 +0000 (+0200) Subject: Merge branch 't/timing_improvements' X-Git-Tag: v0.4.8~10 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=5ff2b0b3a6f9242033c436fb8272245f5594dd8c;hp=0bd00f72312a79900d211e735b135c220ddd0d68 Merge branch 't/timing_improvements' --- diff --git a/NEWS b/NEWS index 7d330ce4..49747ccb 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,9 @@ are the highlights of this release. to replace 5 lines in the config file (one for each audio format) by one single line. See the manual for details. - Compiles cleanly also with llvm/clang. + - Corrupt mp3 files are handled more gracefully. + - sched: Optimized zero timeouts. + - vss timeout cleanups. -------------------------------------- 0.4.7 (2011-06-01) "infinite rollback" diff --git a/afh.h b/afh.h index 8b747f8a..0486bd77 100644 --- a/afh.h +++ b/afh.h @@ -80,7 +80,7 @@ struct audio_format_handler { /** * Check if this audio format handler can handle the file. * - * This is a pointer to a function returning whether a given file is + * This is a pointer to a function returning whether a given file is * valid for this audio format. A negative return value indicates that * this audio format handler is unable to decode the given file. On * success, the function must return a positive value and fill in the @@ -90,7 +90,7 @@ struct audio_format_handler { */ int (*get_file_info)(char *map, size_t numbytes, int fd, struct afh_info *afi); - + /** Optional, used for header-rewriting. See \ref afh_get_header(). */ void (*get_header)(void *map, size_t mapsize, char **buf, size_t *len); }; @@ -101,7 +101,6 @@ int compute_afhi(const char *path, char *data, size_t size, const char *audio_format_name(int); void afh_get_chunk(long unsigned chunk_num, struct afh_info *afhi, void *map, const char **buf, size_t *len); -uint32_t afh_get_largest_chunk_size(struct afh_info *afhi); void afh_get_header(struct afh_info *afhi, uint8_t audio_format_id, void *map, size_t mapsize, char **buf, size_t *len); void afh_free_header(char *header_buf, uint8_t audio_format_id); diff --git a/afh_common.c b/afh_common.c index 76967413..44a6fd62 100644 --- a/afh_common.c +++ b/afh_common.c @@ -213,7 +213,7 @@ success: * * \param i The audio format number. * - * This returns a pointer to statically allocated memory so it + * \return This returns a pointer to statically allocated memory so it * must not be freed by the caller. */ const char *audio_format_name(int i) diff --git a/client.h b/client.h index 2d63c410..28b786a4 100644 --- a/client.h +++ b/client.h @@ -42,6 +42,7 @@ struct client_task { char *user; /** The client task structure. */ struct task task; + /** The buffer tree node of the client task. */ struct btr_node *btrn; }; diff --git a/crypt_common.c b/crypt_common.c index 8fac0dc7..8de346c7 100644 --- a/crypt_common.c +++ b/crypt_common.c @@ -86,7 +86,7 @@ int base64_decode(char const *src, unsigned char *target, size_t targsize) break; pos = strchr(Base64, ch); - if (pos == 0) /* A non-base64 character. */ + if (pos == NULL) /* A non-base64 character. */ return -E_BASE64; switch (state) { diff --git a/filter_common.c b/filter_common.c index 4b1a45de..1233c9ad 100644 --- a/filter_common.c +++ b/filter_common.c @@ -169,6 +169,8 @@ void generic_filter_pre_select(struct sched *s, struct task *t) * \param sample_rate Known to the decoder. * \param channels Known to the decoder. * \param result Ascii representation on the answer is stored here. + * + * \return Standard. */ int decoder_execute(const char *cmd, unsigned sample_rate, unsigned channels, char **result) diff --git a/gcrypt.c b/gcrypt.c index b40b7b6e..f825f6c6 100644 --- a/gcrypt.c +++ b/gcrypt.c @@ -164,7 +164,7 @@ static void pad_oaep(unsigned char *in, size_t in_len, unsigned char *out, /* rfc 3447, section 7.1.2 */ static int unpad_oaep(unsigned char *in, size_t in_len, unsigned char *out, size_t *out_len) -{ int ret; +{ unsigned char *masked_seed = in + 1; unsigned char *db = in + 1 + HASH_SIZE; unsigned char seed[HASH_SIZE], seed_mask[HASH_SIZE]; @@ -189,7 +189,7 @@ static int unpad_oaep(unsigned char *in, size_t in_len, unsigned char *out, p++; *out_len = in + in_len - p; memcpy(out, p, *out_len); - return ret; + return 1; } struct asymmetric_key { @@ -763,7 +763,9 @@ static int decode_rsa(gcry_sexp_t sexp, int key_size, unsigned char *outbuf, PARA_DEBUG_LOG("decrypted buffer before unpad (%d bytes):\n", key_size); dump_buffer("non-unpadded decrypted buffer", oaep_buf, key_size);; - unpad_oaep(oaep_buf, key_size, outbuf, nbytes); + ret = unpad_oaep(oaep_buf, key_size, outbuf, nbytes); + if (ret < 0) + goto out_mpi_release; PARA_DEBUG_LOG("decrypted buffer after unpad (%zu bytes):\n", *nbytes); dump_buffer("unpadded decrypted buffer", outbuf, *nbytes);; diff --git a/gui.c b/gui.c index c8721565..a3c07ec7 100644 --- a/gui.c +++ b/gui.c @@ -99,7 +99,7 @@ static void com_reread_conf(void); static void com_enlarge_top_win(void); static void com_shrink_top_win(void); static void com_version(void); -static void com_quit(void); +__noreturn static void com_quit(void); static void com_refresh(void); static void com_ll_incr(void); static void com_ll_decr(void); diff --git a/mp3dec_filter.c b/mp3dec_filter.c index 82f01418..6982f264 100644 --- a/mp3dec_filter.c +++ b/mp3dec_filter.c @@ -24,15 +24,6 @@ #define MAD_TO_SHORT(f) (f) >= MAD_F_ONE? SHRT_MAX :\ (f) <= -MAD_F_ONE? -SHRT_MAX : (signed short) ((f) >> (MAD_F_FRACBITS - 15)) -/** State of the decoding process. */ -enum mp3dec_flags { - /** Bad main_data_begin pointer encounterd. */ - MP3DEC_FLAG_BAD_DATA = 1, - /** Some output has already been produced. */ - MP3DEC_FLAG_DECODE_STARTED = 2, - MP3DEC_FLAG_NEED_MORE = 4, -}; - /** Data specific to the mp3dec filter. */ struct private_mp3dec_data { /** Information on the current mp3 stream. */ @@ -41,57 +32,22 @@ struct private_mp3dec_data { struct mad_frame frame; /** Contains the PCM output. */ struct mad_synth synth; - /** See \ref mp3dec_flags. */ - unsigned flags; - /** Defer decoding until this time. */ - struct timeval stream_start_barrier; - /** Wait until this many input bytes are available. */ - size_t input_len_barrier; /** The number of channels of the current stream. */ unsigned int channels; /** Current sample rate in Hz. */ unsigned int sample_rate; }; -static int need_bad_data_delay(struct private_mp3dec_data *pmd, - size_t bytes_available) -{ - if (!(pmd->flags & MP3DEC_FLAG_BAD_DATA)) - return 0; - if (pmd->flags & MP3DEC_FLAG_DECODE_STARTED) - return 0; - if (bytes_available >= pmd->input_len_barrier) - return 0; - if (tv_diff(now, &pmd->stream_start_barrier, NULL) > 0) - return 0; - return 1; -} - -/* - * Returns negative on serious errors, zero if the error should be ignored and - * positive on bad data pointer errors at stream start. - */ -static int handle_decode_error(struct private_mp3dec_data *pmd, size_t len) +/* Returns negative on serious errors. */ +static int handle_decode_error(struct private_mp3dec_data *pmd) { - const struct timeval delay = {0, 60 * 1000}; if (!MAD_RECOVERABLE(pmd->stream.error) && pmd->stream.error != MAD_ERROR_BUFLEN) { PARA_ERROR_LOG("%s\n", mad_stream_errorstr(&pmd->stream)); return -E_MAD_FRAME_DECODE; } PARA_DEBUG_LOG("%s\n", mad_stream_errorstr(&pmd->stream)); - if (pmd->stream.error != MAD_ERROR_BADDATAPTR) - return 0; - if (pmd->flags & MP3DEC_FLAG_DECODE_STARTED) - return 0; - /* - * Bad data pointer at stream start. Defer decoding until the amount of - * data we are about to skip is available again, but wait at most 60ms. - */ - pmd->flags |= MP3DEC_FLAG_BAD_DATA; - pmd->input_len_barrier = len; - tv_add(now, &delay, &pmd->stream_start_barrier); - return 1; + return 0; } static size_t used_mad_buffer_bytes(struct mad_stream *s, size_t max) @@ -124,7 +80,7 @@ static void mp3dec_post_select(__a_unused struct sched *s, struct task *t) int i, ret; struct private_mp3dec_data *pmd = fn->private_data; struct btr_node *btrn = fn->btrn; - size_t loaded, used, len, iqs; + size_t loaded = 0, used, len, iqs; char *inbuffer, *outbuffer; next_buffer: @@ -134,8 +90,6 @@ next_buffer: ret = btr_node_status(btrn, fn->min_iqs, BTR_NT_INTERNAL); if (ret < 0) goto err; - if (need_bad_data_delay(pmd, iqs)) - return; if (ret == 0) return; btr_merge(btrn, fn->min_iqs); @@ -158,37 +112,37 @@ next_frame: goto err; } fn->min_iqs += 100; + } + if (loaded == 0) goto next_buffer; - } else if (pmd->stream.error != MAD_ERROR_LOSTSYNC) - PARA_DEBUG_LOG("header decode: %s\n", - mad_stream_errorstr(&pmd->stream)); - goto next_buffer; + return; } fn->min_iqs = 0; pmd->sample_rate = pmd->frame.header.samplerate; pmd->channels = MAD_NCHANNELS(&pmd->frame.header); +decode: ret = mad_frame_decode(&pmd->frame, &pmd->stream); if (ret != 0) { - PARA_INFO_LOG("frame decode: %s\n", mad_stream_errorstr(&pmd->stream)); - used = used_mad_buffer_bytes(&pmd->stream, len); - ret = handle_decode_error(pmd, used); - btr_consume(btrn, used); + ret = handle_decode_error(pmd); if (ret < 0) goto err; - if (ret == 0) - goto next_buffer; + mad_stream_sync(&pmd->stream); + if (pmd->stream.error == MAD_ERROR_BUFLEN) + return; + if (pmd->stream.error != MAD_ERROR_BADDATAPTR) + goto decode; + used = used_mad_buffer_bytes(&pmd->stream, len); + btr_consume(btrn, used); return; } mad_synth_frame(&pmd->synth, &pmd->frame); - pmd->flags |= MP3DEC_FLAG_DECODE_STARTED; - - outbuffer = para_malloc(pmd->synth.pcm.length * 4); + outbuffer = para_malloc(pmd->synth.pcm.length * 2 * pmd->channels); loaded = 0; for (i = 0; i < pmd->synth.pcm.length; i++) { int sample = MAD_TO_SHORT(pmd->synth.pcm.samples[0][i]); write_int16_host_endian(outbuffer + loaded, sample); loaded += 2; - if (MAD_NCHANNELS(&pmd->frame.header) == 2) { /* stereo */ + if (pmd->channels == 2) { /* stereo */ sample = MAD_TO_SHORT(pmd->synth.pcm.samples[1][i]); write_int16_host_endian(outbuffer + loaded, sample); loaded += 2; diff --git a/net.c b/net.c index ae596e5a..9b58e225 100644 --- a/net.c +++ b/net.c @@ -576,8 +576,12 @@ static inline int estimated_header_overhead(const int af_type) } /** - * Maximum transport-layer message size (MMS_S) as per RFC 1122, 3.3.3 - * Socket must be connected. + * Get the maximum transport-layer message size (MMS_S). + * + * The socket must be connected. See RFC 1122, 3.3.3. + * + * \return If the protocol familiy could not be determined, \p AF_INET is + * assumed. */ int generic_max_transport_msg_size(int sockfd) { diff --git a/sched.c b/sched.c index 65176bd9..385dde61 100644 --- a/sched.c +++ b/sched.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "para.h" #include "ipc.h" @@ -46,24 +47,25 @@ static void unregister_task(struct task *t) list_del(&t->post_select_node); } +static inline bool timeout_is_zero(struct sched *s) +{ + struct timeval *tv = &s->select_timeout; + return tv->tv_sec == 0 && tv->tv_usec == 0; +} + static void sched_preselect(struct sched *s) { struct task *t, *tmp; list_for_each_entry_safe(t, tmp, &pre_select_list, pre_select_node) { - if (t->pre_select) - t->pre_select(s, t); -// PARA_INFO_LOG("%s \n", t->status); - if (t->error >= 0) + if (t->error < 0) { + unregister_task(t); continue; - /* - * We have to check whether the list is empty because the call - * to ->pre_select() might have called sched_shutdown(). In - * this case t has been unregistered already, so we must not - * unregister it again. - */ - if (list_empty(&pre_select_list)) - return; - unregister_task(t); + } + if (!t->pre_select) + continue; + t->pre_select(s, t); + if (timeout_is_zero(s)) + break; } } @@ -97,7 +99,12 @@ static void sched_post_select(struct sched *s) // PARA_INFO_LOG("%s: %d\n", t->status, t->ret); if (t->error >= 0) continue; - /* nec., see sched_preselect() */ + /* + * We have to check whether the list is empty because the call + * to ->post_select() might have called sched_shutdown(). In + * this case t has been unregistered already, so we must not + * unregister it again. + */ if (list_empty(&post_select_list)) return; unregister_task(t); @@ -136,21 +143,23 @@ again: sched_preselect(s); if (list_empty(&pre_select_list) && list_empty(&post_select_list)) return 0; - ret = s->select_function(s->max_fileno + 1, &s->rfds, &s->wfds, - &s->select_timeout); - if (ret < 0) - return ret; - if (ret == 0) { - /* - * APUE: Be careful not to check the descriptor sets on return - * unless the return value is greater than zero. The return - * state of the descriptor sets is implementation dependent if - * either a signal is caught or the timer expires. - */ - FD_ZERO(&s->rfds); - FD_ZERO(&s->wfds); + if (!timeout_is_zero(s)) { + ret = s->select_function(s->max_fileno + 1, &s->rfds, &s->wfds, + &s->select_timeout); + if (ret < 0) + return ret; + if (ret == 0) { + /* + * APUE: Be careful not to check the descriptor sets on return + * unless the return value is greater than zero. The return + * state of the descriptor sets is implementation dependent if + * either a signal is caught or the timer expires. + */ + FD_ZERO(&s->rfds); + FD_ZERO(&s->wfds); + } + gettimeofday(now, NULL); } - gettimeofday(now, NULL); sched_post_select(s); if (list_empty(&pre_select_list) && list_empty(&post_select_list)) return 0; @@ -197,7 +206,8 @@ void register_task(struct task *t) * Unregister all tasks. * * This will cause \a schedule() to return immediately because both the - * \a pre_select_list and the \a post_select_list are empty. + * \a pre_select_list and the \a post_select_list are empty. This function + * must be called from the post_select (rather than the pre_select) method. */ void sched_shutdown(void) { @@ -258,8 +268,7 @@ char *get_task_list(void) */ void sched_min_delay(struct sched *s) { - s->select_timeout.tv_sec = 0; - s->select_timeout.tv_usec = 1; + s->select_timeout.tv_sec = s->select_timeout.tv_usec = 0; } /** diff --git a/web/manual.m4 b/web/manual.m4 index f7071167..7163bb7b 100644 --- a/web/manual.m4 +++ b/web/manual.m4 @@ -498,12 +498,13 @@ as follows: - para_client connects to para_server and sends an authentication request for a user. It does so by connecting - to para_server, TCP 2990, the control port of para_server. + to TCP port 2990 of the server host. This port is called the + para_server _control port_. - para_server accepts the connection and forks a child process - which is supposed to handle the connection. The parent process - keeps listening on the control port while the child process - (also called para_server below) continues as follows. + which handles the incoming request. The parent process keeps + listening on the control port while the child process (also + called para_server below) continues as follows. - para_server loads the RSA public key of that user, fills a fixed-length buffer with random bytes, encrypts that buffer @@ -513,7 +514,7 @@ as follows: session key. - para_client receives the encrypted buffer and decrypts it - using the user's private key, thereby obtaining the challenge + with the user's private key, thereby obtaining the challenge buffer and the session key. It sends the SHA1 hash value of the challenge back to para_server and stores the session key for further use. @@ -542,8 +543,8 @@ file, below). The user_list file ~~~~~~~~~~~~~~~~~~ -At startup para_server reads the user list file which must contain -one line per user. The default location of the user list file may be +At startup para_server reads the user list file which contains one +line per user. The default location of the user list file may be changed with the --user_list option. There should be at least one user in this file. Each user must have @@ -610,10 +611,11 @@ known audio files to those which satisfy certain criteria. It also maintains tables containing images (e.g. album cover art) and lyrics that can be associated with one or more audio files. -AFS uses libosl, the object storage layer, as the backend library -for storing information on audio files, playlists, etc. This library -offers functionality similar to a relational database, but is much -more lightweight than a full database backend. +AFS uses XREFERENCE(http://systemlinux.org/~maan/osl/, libosl), the +object storage layer library, as the backend library for storing +information on audio files, playlists, etc. This library offers +functionality similar to a relational database, but is much more +lightweight than a full database backend. In this chapter we sketch the setup of the REFERENCE(The AFS process, AFS process) during server startup and proceed with the description @@ -623,7 +625,7 @@ and moods) explains these two audio file selection mechanisms in detail and contains pratical examples. The way REFERENCE(File renames and content changes, file renames and content changes) are detected is discussed briefly before the REFERENCE(Troubleshooting, -Troubleshooting) section which concludes the chapter. +Troubleshooting) section concludes the chapter. The AFS process ~~~~~~~~~~~~~~~ @@ -726,7 +728,7 @@ Similarly, the "test" bit can be removed from an audio file with para_client setatt test- /path/to/the/audio/file Instead of a path you may use a shell wildcard pattern. The attribute -is applied to all audio files matching that pattern: +is applied to all audio files matching this pattern: para_client setatt test+ '/test/directory/*' @@ -782,7 +784,7 @@ can be used. Note that the images and lyrics are not interpreted at all, and also the playlist and the mood blobs are only investigated when the mood -or playlist is activated by using the select command. +or playlist is activated with the select command. *The score table* @@ -798,7 +800,9 @@ next. While doing so, it computes the new score and updates the last_played and the num_played fields in the audio file table. The score table is recomputed by the select command which loads a -new mood or playlist. +mood or playlist. Audio files are chosen for streaming from the rows +of the score table on a highest-score-first basis. + Playlists and moods ~~~~~~~~~~~~~~~~~~~ @@ -810,17 +814,12 @@ terms of attributes and other type of information available in the audio file table. As an example, a mood can define a filename pattern, which is then matched against the names of audio files in the table. -Selecting a mood or playlist means the generation of a ranking -(a score table) for the set of admissible files. Audio files are -then selected on a highest-score-first basis. The score table is -recomputed at the moment the mood or playlist is selected. - *Playlists* Playlists are accommodated in the playlist table of the afs database, -using the aforementioned blob format for tables. A new filelist is -created using the addpl command, by specifying the full (absolute) -paths of all desired audio files, separated by newlines. For example +using the aforementioned blob format for tables. A new playlist is +created with the addpl command by specifying the full (absolute) +paths of all desired audio files, separated by newlines. Example: find /my/mp3/dir -name "*.mp3" | para addpl my_playlist @@ -840,7 +839,7 @@ A mood consists of a unique name and its *mood definition*, which is a set of *mood lines* containing expressions in terms of attributes and other data contained in the database. -At any time, at most one mood can be *active* which means that +At any time at most one mood can be *active* which means that para_server is going to select only files from that subset of admissible files. @@ -950,7 +949,7 @@ The year tag is special as its value is undefined if the audio file has no year tag or the content of the year tag is not a number. Such audio files never match. Another difference is the special treatment if the year tag is a two-digit number. In this case either 1900 or -2000 are added to the tag value depending on whether the number is +2000 is added to the tag value, depending on whether the number is greater than 2000 plus the current year. @@ -1886,7 +1885,7 @@ the tip of topic branches you are interested in from the output of "git log next"). You should be able to safely build on top of them. However, at times "next" will be rebuilt from the tip of "master" to -get rid of merge commits that will never be in "master. The commit +get rid of merge commits that will never be in "master". The commit that replaces "next" will usually have the identical tree, but it will have different ancestry from the tip of "master". diff --git a/wma_common.c b/wma_common.c index 137d288d..1dde8350 100644 --- a/wma_common.c +++ b/wma_common.c @@ -116,7 +116,7 @@ int read_asf_header(const char *buf, int loaded, struct asf_header_info *ahi) return 1; } -const uint8_t log2_tab[256] = { +static const uint8_t log2_tab[256] = { 0, 0, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,