11 days agoMerge branch 'refs/heads/t/ll' master
Andre Noll [Sun, 18 Sep 2022 14:28:33 +0000 (16:28 +0200)]
Merge branch 'refs/heads/t/ll'

Two little cleanups related to the logging facility and two commits
which add the ll command to para_server and para_audiod.

The merge resulted in a conflict in afs.c due to the earlier merge of
the poll topic branch which replaced all calls to select() by calls
to poll(). The implementation of the ll server command introduced a
new caller of select(), afs_select(), which needs to be replaced by
afs_poll() to resolve the conflict.

* refs/heads/t/ll:
  New server command: ll to change the log level at runtime.
  New audiod command: ll to change the log level at runtime.
  daemon: Kill get_loglevel_by_name().
  server/audiod: Don't parse loglevel argument unnecessarily.

2 weeks agoMerge branch 'refs/heads/t/poll'
Andre Noll [Sun, 11 Sep 2022 14:34:12 +0000 (16:34 +0200)]
Merge branch 'refs/heads/t/poll'

This series modifies all calls to select(2) to use poll(2) instead in
order to avoid the known shortcomings of the select API, in particular
its limit of at most 1024 file descriptors and the fact that fds above
1023 cannot be monitored with select(2) even if fewer than 1024 fds
are open.

The first patches of the series prepare this switch, converting the
easy cases, hiding select specific data structures such as fd sets,
and adjusting function names and documentation. The crucial commit
is the last one. See its rather verbose log message for details.

* refs/heads/t/poll:
  Switch from select(2) to poll(2).
  Rename ->{pre,post}_select methods to ->{pre,post}_monitor.
  Misc documentation cleanups related to select().
  stdin/stdout: Streamline documentation of {pre,post}_select().
  Consolidate receiver/filter/writer {pre,post}_select() docs.
  Hide implementation of para_fd_set().
  send: Avoid select-specific arguments in {pre,post}_select().
  sched: Introduce sched_{read,write}_ok().
  audiod: Rename handle_connect().
  net: Drop fd_set parameter from para_accept().
  fd: Drop fd_set parameter from read_nonblock() and friends.
  interactive: Avoid select(2) in input_available().
  fd.c: Prefer poll(2) over select(2) for write_ok().
  sched: Use integer value for select timeout.

3 weeks agoMerge branch 'refs/heads/t/mp4ff'
Andre Noll [Sat, 3 Sep 2022 14:29:10 +0000 (16:29 +0200)]
Merge branch 'refs/heads/t/mp4ff'

This 140 patch behemoth adds a stripped down copy of libmp4ff to the
repo. This has become necessary because the library was dropped from
the faad project.

The series starts with a patch which adds an unmodified copy of the
relevant parts of libmp4ff to the repo. All code is combined in a
single file, mp4.c, which contains approximately 2K lines of code. The
public API is defined in the new mp4.h.

The remaining patches clean up the two new files, simplifying and
removing large parts of it. Some of the patches modify the API,
and those require small changes to the aac audio format handler,
which also becomes simpler due to these changes. The aac decoder,
however, is not touched in this series.

* refs/heads/t/mp4ff: (140 commits)
  mp4: Don't abort on truncated files.
  mp4: Document the purpose of each atom.
  mp4: Doxify the public API.
  mp4: Check for missing metadata also for regular opens.
  mp4: Rename mp4_open_read() to mp4_open().
  mp4: Rename mp4_meta_update() to mp4_update_meta().
  mp4: Simplify mp4_num_samples().
  mp4: Reject files with zero time scale.
  mp4: Assorted trivial cleanups.
  mp4: Remove ->len member of struct mp4_tag.
  mp4: Fix possible memory leak on errors.
  mp4: Return proper types for sample rate and count.
  mp4: Fail early on invalid sample rate or sample count.
  mp4: Remove E_MP4_BAD_CHANNEL_COUNT.
  mp4: Improve mp4_get_sample_size().
  mp4: Make sample number be an unsigned parameter.
  mp4: Check the return value of ->truncate().
  mp4: Make most loop variables unsigned.
  mp4: Replace the five tag value functions by a single one.
  mp4: Provide proper error codes for all errors.

4 weeks agoMerge branch 'refs/heads/t/autogen'
Andre Noll [Wed, 31 Aug 2022 13:04:00 +0000 (15:04 +0200)]
Merge branch 'refs/heads/t/autogen'

A single patch which removes everything from except the
autoconf and autoheader commands.

* refs/heads/t/autogen:

4 weeks agoSwitch from select(2) to poll(2).
Andre Noll [Sun, 10 Oct 2021 20:18:24 +0000 (22:18 +0200)]
Switch from select(2) to poll(2).

The select(2) API is kind of obsolete because it does not work for
file descriptors greater or equal than 1024, The general advice is
to switch to poll(2), which offers equivalent functionality and does
not suffer from this restriction. This patch implements this switch.

The fd sets of select(2) have one nice feature: One can determine in
O(1) time whether the bit for a given fd is turned on in an fd set.

For poll(2), the monitored file descriptors are organized in an array
of struct pollfd. Without information about the given fd's index in the
pollfd array, one can only perform a linear search which requires O(n)
time, with n being the number of fds being watched. Since this would
have to be done for each fd, the running time becomes quadratic in
the number of monitored fds, which is bad. Keeping the pollfd array
sorted would reduce that to n * log(n) at the cost of additional work
at insert time.

This patch implements a different approach. The scheduler now maintains
an additional array of unsigned integers which map fds to indices
into the pollfd array. This new index array is transparent to the
individual tasks, which still simply pass one or more fds from their
->pre_monitor() method to the scheduler. The length of the index array
equals the highest fd given. This might become prohibitive in theory,
but should not be an issue for the time being.

Care needs to be taken in order to deal with callers which ask for
the readiness of an fd without having called sched_monitor_readfd() or
sched_monitor_writefd() in the ->pre_monitor() step. Before the patch,
thanks to the FD_ZERO() call at the beginning of each iteration of
the scheduler's main loop, both sched_read_ok() and sched_write_ok()
returned false for fds which were not asked to be watched. We need
to keep it this way for a seamless transition.

We achieve this by replacing the FD_ZERO() call by a memset(3) call
which fills the index array with 0xff bytes. Both sched_read_ok() and
sched_write_ok() call the new get_revents() helper, where we check the
fd argument against the allocation sizes of the two arrays. If either
function is called with an fd that was not asked to be monitored in
the ->pre_monitor() step, the checks notice that the index of this
fd, 0xffffffff, is larger than the highest open fd and we return
"not ready for I/O".

Another issue is the case where the same file descriptor is submitted
twice in ->pre_monitor() to check for readiness with respect to both
reading and writing. The code in client_comon.c currently does that.
To keep it working, the scheduler needs to detect this case and re-use
the existing slot in both arrays.

5 weeks agoRename ->{pre,post}_select methods to ->{pre,post}_monitor.
Andre Noll [Sun, 10 Oct 2021 16:10:09 +0000 (18:10 +0200)]
Rename ->{pre,post}_select methods to ->{pre,post}_monitor.

The word "monitor" is neutral and continues to be correct after the
switch from select(2) to poll(2).

Pure rename, nothing to see here.

5 weeks agoMisc documentation cleanups related to select().
Andre Noll [Thu, 7 Oct 2021 20:16:09 +0000 (22:16 +0200)]
Misc documentation cleanups related to select().

Assorted comment cleanups which avoid to talk about select(2) and
fd sets. No code changes.

5 weeks agostdin/stdout: Streamline documentation of {pre,post}_select().
Andre Noll [Thu, 7 Oct 2021 19:22:59 +0000 (21:22 +0200)]
stdin/stdout: Streamline documentation of {pre,post}_select().

Don't state the obvious and avoid talking about fd sets.

5 weeks agoConsolidate receiver/filter/writer {pre,post}_select() docs.
Andre Noll [Thu, 7 Oct 2021 17:22:47 +0000 (19:22 +0200)]
Consolidate receiver/filter/writer {pre,post}_select() docs.

The documentation of these three ->pre_select() and ->post_select()
methods overlapped quite a bit. Some comments stated general properties
of the sched API which fit better in the documentation of sched.h,
so move these bits there.

Improve the text a bit while at it and avoid talking about select(2)
and fd sets as these are implementation details. Instead, focus on
the general concept of fd monitoring.

Pure comment cleanups, no code changes.

5 weeks agoHide implementation of para_fd_set().
Andre Noll [Sun, 3 Oct 2021 19:52:02 +0000 (21:52 +0200)]
Hide implementation of para_fd_set().

This preparatory patch for replacing select() renames para_fd_set()
to sched_fd_set(), moves it to sched.c and makes it static. All
users are modified to call either of the two new public functions
sched_monitor_{read,write}fd() which take a pointer to struct sched
rather than an fd set pointer.

5 weeks agosend: Avoid select-specific arguments in {pre,post}_select().
Andre Noll [Sun, 3 Oct 2021 19:37:40 +0000 (21:37 +0200)]
send: Avoid select-specific arguments in {pre,post}_select().

Just pass a pointer to struct sched instead of the fd sets. Since
two of the prototypes declared in send.h now refer to this structure,
sched.h must be included before send.h.

The udp sender implements neither ->pre_select() nor ->post_select(),
so we only need to fix the order in which send.h and sched.h are

5 weeks agosched: Introduce sched_{read,write}_ok().
Andre Noll [Thu, 30 Sep 2021 20:35:10 +0000 (22:35 +0200)]
sched: Introduce sched_{read,write}_ok().

Two trivial wrappers for FD_ISSET() which hide the fact that we're
still using the select(2) API.

5 weeks agoaudiod: Rename handle_connect().
Andre Noll [Tue, 12 Oct 2021 17:50:53 +0000 (19:50 +0200)]
audiod: Rename handle_connect().

Now that it has the same signature as para_server's handle_connect(),
doxygen gets confused and complains as follows:

audiod_command.c:359: warning: argument 'accept_fd' of command @param is not found in the argument list of handle_connect(int fd)

This is a false positive, but since "handle_connect" is not a very
descriptive name for a public function in the first place, let's
rename it.

5 weeks agonet: Drop fd_set parameter from para_accept().
Andre Noll [Thu, 30 Sep 2021 20:18:43 +0000 (22:18 +0200)]
net: Drop fd_set parameter from para_accept().

As for read_nonblock(), the parameter is dispensable because it is
only used for an optimization to avoid a system call. Get rid of it
because it hinders the conversion from select(2) to poll(2).

5 weeks agofd: Drop fd_set parameter from read_nonblock() and friends.
Andre Noll [Thu, 30 Sep 2021 19:46:58 +0000 (21:46 +0200)]
fd: Drop fd_set parameter from read_nonblock() and friends.

This parameter is not necessary because its only purpose is to
avoid the readv(2) system call in case it would likely return EAGAIN
because we just called select(2) which reported that there is no data
to read. Since the parameter is an obstacle for the conversion of
the code base from select(2) to poll(2), get rid of it for the time
being. If needed we can add back an equivalent optimization which
checks for POLLIN after the conversion.

5 weeks agointeractive: Avoid select(2) in input_available().
Andre Noll [Wed, 29 Sep 2021 20:29:51 +0000 (22:29 +0200)]
interactive: Avoid select(2) in input_available().

In analogy to write_ok(), introduce read_ok() which uses poll(2)
rather than select(2). To avoid duplications, abstract out the common
code to the new xpoll() helper.

We could avoid the timeout parameter of xpoll() at this point because
both callers call it with a zero timeout (causing poll() to return
immediately), but later patches introduce other callers which specify
non-zero timeouts.

5 weeks agofd.c: Prefer poll(2) over select(2) for write_ok().
Andre Noll [Wed, 29 Sep 2021 20:07:13 +0000 (22:07 +0200)]
fd.c: Prefer poll(2) over select(2) for write_ok().

This is easy to do and avoids the old and well-known shortcomings of
select(2). See

for a short discussion, or the references in the log message of commit
e4a403876d2c of the man-pages repository.

The linux poll manpage says:

On some other UNIX systems, poll() can fail with the error EAGAIN if
the system fails to allocate kernel-internal resources, rather than
ENOMEM as Linux does. POSIX permits this behavior. Portable programs
may wish to check for EAGAIN and loop, just as with EINTR.

We do not follow this approach since failing the call in the out of
memory case seems to be the right thing to do while busy looping
without trying to free memory between the calls is not likely to
help. Also, looping on EAGAIN would be inconsistent since in the
OOM case the code would fail on Linux but loop on those other UNIX
systems. To be consistent, one must check for both EAGAIN and ENOMEM.

5 weeks agosched: Use integer value for select timeout.
Andre Noll [Fri, 24 Sep 2021 16:11:06 +0000 (18:11 +0200)]
sched: Use integer value for select timeout.

This modifies the public struct sched so that users pass in the
default timeout as an integer value in milliseconds rather than
a struct timeval. This simplifies the code a little and eases the
transition from select(2) to poll(2) because poll(2) also takes a
plain integer for the timeout.

Since para_select() of fd.c now calls ms2tv() to convert the timeout
back to a struct timeval, all executables which link with fd.o must
also link with time.o. This was not the case for para_mixer and
para_audioc, so needs to be adjusted accordingly.

5 weeks agopara_play: Avoid invalid time display on pause.
Andre Noll [Wed, 27 Jul 2022 13:27:14 +0000 (15:27 +0200)]
para_play: Avoid invalid time display on pause.

When para_play enters pause mode, playback is stopped by notifying
the writer node, causing it to terminate in the next iteration of the
main loop. Until then the play time is computed incorrectly because
we add the running time of the moribund writer node to the start time
computed from the *new* start chunk set in com_pause().

Fortunately, the fix is simple. We just need to enqueue a reposition
request in the same way the ff and jmp commands do.

5 weeks agoMerge branch 'maint'
Andre Noll [Sun, 21 Aug 2022 15:16:32 +0000 (17:16 +0200)]
Merge branch 'maint'

Two fixes for para_play.

5 weeks agoi9e: Fix invalid key handling. maint
Andre Noll [Mon, 25 Jul 2022 22:08:43 +0000 (00:08 +0200)]
i9e: Fix invalid key handling.

If an unmapped key is pressed repeatedly, we store the key sequence in
a 32 byte buffer until there is no more space left in the buffer. Then
we terminate the process with

para_play: interactive.c:304: i9e_post_monitor: Assertion `len < sizeof(i9ep->key_sequence) - 1' failed.

This is not a nice way to deal with invalid input, so be a bit more
graceful and discard the buffer when it is full or when there is no
further input available at the moment.

5 weeks agoi9e: Fix braino in i9e_post_select().
Andre Noll [Mon, 25 Jul 2022 21:20:11 +0000 (23:20 +0200)]
i9e: Fix braino in i9e_post_select().

Due to this bug we mishandled the case where the read() returns zero
to indicate EOF. In this case we stuffed a random character instead
of shutting down the i9e task.

2 months agoaudiod: Demote severity level of command errors.
Andre Noll [Sun, 17 Jul 2022 19:33:24 +0000 (21:33 +0200)]
audiod: Demote severity level of command errors.

handle_connect() returning negative is a normal condition which
occurs, for example, when the user specifies an invalid argument to
a command. Therefore the "notice" severity level is more appropriate
than the current "error" severity.

2 months agoaudiod: Fix time diff warning log message.
Andre Noll [Mon, 13 Jun 2022 18:31:23 +0000 (20:31 +0200)]
audiod: Fix time diff warning log message.

The format string contains %lu to print a long unsigned, but we
possibly multiply the value with -1, which can lead to output like

Jun 13 13:36:37 baader (3) compute_time_diff: time diff jump: 4294808018ms

Fix this by replacing the multiplication with an additional format
string directive to print the leading +/- explicitly. This is easy
since we already have the sign in a variable, and it avoids any
integer conversion/overflow issues.

2 months agoNew server command: ll to change the log level at runtime.
Andre Noll [Tue, 19 Oct 2021 19:41:45 +0000 (21:41 +0200)]
New server command: ll to change the log level at runtime.

This makes use of the infrastructure introduced in the previous patch.
However, the implementation of the ll command for para_server is more
involved than its audiod counterpart because in the server case we
have to tell two different processes (server and afs) to change their
log level while the calling process, the command handler, does not
need to set the loglevel because it is about to exit anyway.

For the inter-process communication we introduce a new field in the
mmd shared memory area so that command handlers can read the current
value or set a new value. The log level propagates from there via
daemon_set_loglevel() to the server and afs processes during each
iteration of the scheduler loop where para_log() will pick it up to
set the log level threshold for subsequent log events.

The si command handler currently refers to the argument of the
--loglevel server option to include the log level in its output. With
dynamic log levels this no longer works because it always prints the
value from the command line or the config file rather than the run
time log level. Since the new ll command also prints the loglevel
when it is executed with no arguments, we simply remove this line
from the si output and hope that nobody cares.

The si command handler was the last user of the ENUM_STRING_VAL macro
in command.c. Removing the macro also allows us to make CMD_PTR local
to server.c and to remove the lopsub definitions of the server suite
from command.c. However, we still include the lopsub definitions of
the server *command* suite (server_cmd.lsg.h) of course.

We let any authenticated user run the command with no arguments to
report the current loglevel but require full privileges to change
the loglevel. Thus, the check for sufficient privileges needs to be
performed in the command handler.

2 months agoNew audiod command: ll to change the log level at runtime.
Andre Noll [Tue, 19 Oct 2021 20:19:25 +0000 (22:19 +0200)]
New audiod command: ll to change the log level at runtime.

The new public daemon_get_loglevel() is needed in the zero argument
case. Otherwise, the ll command handler parses the argument and calls

The lopsub stanza for the subcommand is stored in a separate file
which is currently only included by the lopsub suite for para_audiod,
but will be included as well by the server suite.

For similar reasons we implement the completer as a generic public
function, i9e_ll_completer(), although it only has one caller in
audioc.c. Another caller follows when the ll server command is added.

2 months agodaemon: Kill get_loglevel_by_name().
Andre Noll [Tue, 19 Oct 2021 19:44:14 +0000 (21:44 +0200)]
daemon: Kill get_loglevel_by_name().

Open-code the logic in daemon_set_log_color_or_die() and get the
values from the new SEVERTIES macro rather than duplicating the
severity list in get_loglevel_by_name().

The SEVERTIES macro will turn out to be handy for the ll subcommands of
para_server and para_audiod which are introduced in subsequent commits.

2 months agoserver/audiod: Don't parse loglevel argument unnecessarily.
Andre Noll [Tue, 19 Oct 2021 19:28:30 +0000 (21:28 +0200)]
server/audiod: Don't parse loglevel argument unnecessarily.

Currently the severity string (debug, info, etc.) given to --loglevel
is parsed twice: Once by lopsub, which returns the loglevel as the
index into the array of severity strings. We turn this index into a
string and pass the string to daemon_set_loglevel() which parses the
string again to turn it back into a log level value (which happens
to coincide with the index value).

Clean this up by letting daemon_set_loglevel() receive a log level
value rather than a severity string. This also allows us to remove
the now unused ENUM_STRING_VAL() macro from audiod.c.

3 months agomp4: Don't abort on truncated files.
Andre Noll [Wed, 29 Jun 2022 11:38:02 +0000 (13:38 +0200)]
mp4: Don't abort on truncated files.

If the source file got truncated it may happen that a chunk cannot
be read because the computed file offset is beyond EOF. Currently,
aac_afh_get_chunk() aborts in this case because we assert that the
file offset is within range. Return a proper error code instead and
also change aac_get_file_info() to bail out if aac_afh_get_chunk()
returns negative.

3 months agomp4: Document the purpose of each atom.
Andre Noll [Mon, 30 Aug 2021 16:57:15 +0000 (18:57 +0200)]
mp4: Document the purpose of each atom.

This gives the reader a rough idea about the atoms we care about.

3 months agomp4: Doxify the public API.
Andre Noll [Sat, 28 Aug 2021 20:12:41 +0000 (22:12 +0200)]
mp4: Doxify the public API.

This adds doxygen comments to all public functions of the mp4 API
and to the macros and enumerations of mp4.c.

3 months agomp4: Check for missing metadata also for regular opens.
Andre Noll [Sat, 28 Aug 2021 19:15:35 +0000 (21:15 +0200)]
mp4: Check for missing metadata also for regular opens.

Since we allow to update the metadata of a file handle returned by
mp4_open(), we should check for both types of opens that the file
actually contains the udta, meta and ilst atoms.

3 months agomp4: Rename mp4_open_read() to mp4_open().
Andre Noll [Sat, 28 Aug 2021 18:53:23 +0000 (20:53 +0200)]
mp4: Rename mp4_open_read() to mp4_open().

The function may be called with the intention to update the meta tags
later by calling mp4_update_meta(), albeit mp4_open_meta() is cheaper
if the caller only wants to modify the metadata. The old name is thus
slightly misleading, and it's longer.

3 months agomp4: Rename mp4_meta_update() to mp4_update_meta().
Andre Noll [Sat, 28 Aug 2021 18:50:13 +0000 (20:50 +0200)]
mp4: Rename mp4_meta_update() to mp4_update_meta().

Just to be consistent with mp4_open_meta() and friends.

3 months agomp4: Simplify mp4_num_samples().
Andre Noll [Sat, 28 Aug 2021 18:29:06 +0000 (20:29 +0200)]
mp4: Simplify mp4_num_samples().

We don't need to iterate over the entries of the stts_sample_count
array because the number stored in the stsz_sample_count field should
be identical to the sum of the sample counts.

3 months agomp4: Reject files with zero time scale.
Andre Noll [Sat, 28 Aug 2021 16:35:18 +0000 (18:35 +0200)]
mp4: Reject files with zero time scale.

A value of zero indicates a corrupt mp4 file or a missing mdhd
atom. This is fatal because we need to divide by the time scale to
compute the duration of the audio track.

This patch modifies mp4_open_read() to check the value at open time
and fail the operation rather than allowing the open to succeed and
checking the value in mp4_get_duration(),

Only regular opens are affected since we don't look at the mdhd atom
for metadata opens.

3 months agomp4: Assorted trivial cleanups.
Andre Noll [Sat, 28 Aug 2021 14:40:19 +0000 (16:40 +0200)]
mp4: Assorted trivial cleanups.

Avoid C++ comments, use int rather than int32_t as the standard return
type, kill a pointless cast and use plain unsigned rather than uint32_t
for the number of tag items.

3 months agomp4: Remove ->len member of struct mp4_tag.
Andre Noll [Sat, 28 Aug 2021 13:18:41 +0000 (15:18 +0200)]
mp4: Remove ->len member of struct mp4_tag.

It is set but never read.

3 months agomp4: Fix possible memory leak on errors.
Andre Noll [Fri, 27 Aug 2021 17:20:42 +0000 (19:20 +0200)]
mp4: Fix possible memory leak on errors.

If the sanity checks in open_file() fail, we free the mp4 structure
but not the various tables and metadata items we might already have
allocated at this point.

Fix this by calling mp4_close() instead of freeing the mp4 struct
directly. We have to move mp4_close() above open_file() to avoid a
forward declaration.

3 months agomp4: Return proper types for sample rate and count.
Andre Noll [Fri, 27 Aug 2021 17:10:51 +0000 (19:10 +0200)]
mp4: Return proper types for sample rate and count.

The sample rate and the number of samples are stored as 16-bit/32-bit
unsigned integers in the mp4 file, so let mp4_get_sample_rate()
and mp4_num_samples() return these types.

3 months agomp4: Fail early on invalid sample rate or sample count.
Andre Noll [Fri, 27 Aug 2021 17:06:11 +0000 (19:06 +0200)]
mp4: Fail early on invalid sample rate or sample count.

If the sample rate or the sample count happen to be zero, we should
fail the open rather than return success and let the caller deal with
it. This patch moves the corresponding sanity checks from aac_afh.c
to mp4_open_read() of mp4.c. The sample rate is always read while
sample count is skipped for metadata-only opens. So the first check
belongs to the common open_file() while the second check needs to go
to mp4_open_read().

3 months agomp4: Remove E_MP4_BAD_CHANNEL_COUNT.
Andre Noll [Fri, 27 Aug 2021 14:07:24 +0000 (16:07 +0200)]

If the mp4 file does not contain an m4a atom, the channel
count stays at zero and open_file() returns -E_MP4_TRACK in this
case. So the check in aac_afh.c for a non-positive return value from
mp4_get_channel_count() can never trigger. Replace the check by an
assertion and remove the error code.

Also, let mp4_get_channel_count() return uint16_t as the number of
channels is stored as an unsigned 16 bit number in the mp4 file.

3 months agomp4: Improve mp4_get_sample_size().
Andre Noll [Fri, 27 Aug 2021 13:38:25 +0000 (15:38 +0200)]
mp4: Improve mp4_get_sample_size().

Use an unsigned type for the sample number and check that the passed
number is within range. Since the function can fail now, let it return
int and return the sample size via an additional pointer argument.

3 months agomp4: Make sample number be an unsigned parameter.
Andre Noll [Fri, 27 Aug 2021 13:23:24 +0000 (15:23 +0200)]
mp4: Make sample number be an unsigned parameter.

There is no reason to convert the 32-bit unsigned paraslash chunk
number into a signed quantity here, as sample numbers are also stored
as 32 bit unsigned in the mp4 file.

3 months agomp4: Check the return value of ->truncate().
Andre Noll [Thu, 26 Aug 2021 20:11:40 +0000 (22:11 +0200)]
mp4: Check the return value of ->truncate().

This callback is implemented as a simple wrapper for the ftruncate()
system call, which can fail for a number of reasons. Currently the
callback returns unsigned and the return value is ignored. Fortunately,
this is easy to fix.

3 months agomp4: Make most loop variables unsigned.
Andre Noll [Thu, 26 Aug 2021 19:39:31 +0000 (21:39 +0200)]
mp4: Make most loop variables unsigned.

If the loop variable iterates from zero to some number stored in a
variable of unsigned type, the loop variable should be of the same
unsigned type. This was not always the case, and if it was, the loop
variable was sometimes called i, which is confusing because i usually
indicates a signed quantity.

Quoting Andrew Morton:

Doing "unsigned i;" is an act of insane vandalism, punishable by
spending five additional years coding in fortran.

3 months agomp4: Replace the five tag value functions by a single one.
Andre Noll [Thu, 26 Aug 2021 17:57:45 +0000 (19:57 +0200)]
mp4: Replace the five tag value functions by a single one.

It's easier to let the caller pass the tag item string than to
have one caller for each of the five tags of interest. This commit
renames meta_find_by_name() to mp4_get_tag_value(), makes it public
and removes its five callers from mp4.c.

The only user is _aac_afh_get_taginfo() of aac_afh.c, which needs to
be adjusted accordingly. Kill the pointless underscore while at it.

3 months agomp4: Provide proper error codes for all errors.
Andre Noll [Thu, 26 Aug 2021 17:20:49 +0000 (19:20 +0200)]
mp4: Provide proper error codes for all errors.

This changes the few remaining places where we return -1 to indicate
failure by proper error codes which can be turned into a meaningful
error message.

3 months agomp4: Simplify atom_read_header().
Andre Noll [Thu, 26 Aug 2021 17:05:28 +0000 (19:05 +0200)]
mp4: Simplify atom_read_header().

All callers pass non-NULL pointers for the atom size, so the condition
which is removed in this commit is always true.

3 months agomp4: Remove tracks array.
Andre Noll [Thu, 26 Aug 2021 16:36:00 +0000 (18:36 +0200)]
mp4: Remove tracks array.

The mp4 structure currently contains an array of 1024 track pointers
which are initialized to point to track structures allocated as we
encounter tracks. This is kind of wasteful given that we will only
care about audio tracks, and only ever consider the first one.

This patch replaces the pointer array by a single track structure
embedded within struct mp4. Besides the above mentioned memory savings,
this approach allows us to remove a bunch of identical sanity checks
in the atom parsers.

The old code maintained the ->audio_track pointer of struct mp4 to
tell whether we already saw an mp4a atom and thus already allocated
a structure for the corresponding track. We now use a state based
approach with three states instead. The state value determines whether
we have to parse the atom. The first state transition takes place when
the mp4a atom is encountered while the second transition occurs at the
subsequent trak atom, if any. If an atom parser is called while the
state machine is in an unexpected state, we return success rather than
an error code to ignore the atom without failing the whole operation.

4 months agoRemove para_dirname() and para_basename().
Andre Noll [Fri, 11 Mar 2022 18:32:24 +0000 (19:32 +0100)]
Remove para_dirname() and para_basename().

The former has only a single caller, the second only two, open-coding
these is actually simpler and more performant because we no longer
scan each path twice and avoid the temporary copy of the path.

4 months agomp4: Merge read_mp4a() into read_stsd().
Andre Noll [Wed, 25 Aug 2021 16:41:07 +0000 (18:41 +0200)]
mp4: Merge read_mp4a() into read_stsd().

This shortens the code because we already have a track pointer here
and can get rid of the duplicated check for the number of tracks.

The commit also adds the missing error check for the last read
operation, i.e. the one which reads the sample rate.

4 months agomp4: Introduce skip_bytes().
Andre Noll [Tue, 24 Aug 2021 19:37:06 +0000 (21:37 +0200)]
mp4: Introduce skip_bytes().

We often call one of the read_intX() helpers with a NULL result pointer
just to move the file position forward. Calling ->seek() with whence
set to SEEK_CUR is simpler and has the advantage that this operation
cannot fail. If we happen to seek beyond EOF, the next read will
return EOF and we'll abort then.

This patch provides the skip_bytes() helper and replaces all
read_intX(f, NULL) calls by calls to skip_bytes() and removes the
error checking.

Due to this cleanup read_int8() and read_int24() and read_u24_be()
(the latter being an inline function defined in portable_io.h) have
become unused, so remove these as well.

4 months agomp4: Provide whence parameter for the seek callback.
Andre Noll [Tue, 24 Aug 2021 18:47:03 +0000 (20:47 +0200)]
mp4: Provide whence parameter for the seek callback.

This adds a parameter to make ->seek() work like the lseek(2) system
call. This is easy to implement in both the memory-mapped callback
case used to retrieve the file information and the metadata update
case where ->seek() is a trivial wrapper for lseek(2).

With the additional functionality in place we don't need to track
the file size and the current file offset any more in mp4.c as these
values can now be obtained by calling ->seek() with a zero offset and
whence set to SEEK_END and SEEK_CUR, respectively. This also makes
the code more robust against corrupt mp4 files because we no longer
rely on the values from the atom headers to compute the file size.

The way mp4.c calls ->seek() should never cause the underlying lseek(2)
system call to fail. Therefore it suffices to check the return value
only in the callback wrapper and abort on failure.

4 months agomp4: Implement error checking for the write path.
Andre Noll [Tue, 24 Aug 2021 12:49:16 +0000 (14:49 +0200)]
mp4: Implement error checking for the write path.

Although the ->write callback has a return value, it is of unsigned
type and is never checked. Fix this by changing the prototype to
match that of the write(2) system call, check the return value of the
callback in the write_data() wrapper of mp4.c and propagate paraslash
error codes back to aac_afh.c via the public mp4_meta_update().

While at it, handle short writes and EINTR properly, and fix the
indentation of the callback structure in mp4.h.

4 months agomp4: Merge write_int32() into mp4_meta_update().
Andre Noll [Mon, 23 Aug 2021 19:30:14 +0000 (21:30 +0200)]
mp4: Merge write_int32() into mp4_meta_update().

It has only this single caller, and it's short. Use uint8_t instead
of int8_t for the buffer as we do elsewhere and rename the buffer
variable while at it.

4 months agomp4: Simplify mp4_meta_update().
Andre Noll [Mon, 23 Aug 2021 19:12:27 +0000 (21:12 +0200)]
mp4: Simplify mp4_meta_update().

Move duplicated common code out of the if/else branches and kill a
pointless variable.

4 months agomp4: Avoid camel case for members of struct mp4_track.
Andre Noll [Mon, 23 Aug 2021 18:18:54 +0000 (20:18 +0200)]
mp4: Avoid camel case for members of struct mp4_track.

Only three members of struct mp4 are in camel case while all others
follow the underscore convention, which is the standard coding style
of the paraslash code base. Let's be consistent here.

Add comments which indicate the origin of the values stored while
at it.

4 months agomp4: Kill fix_byte_order_32().
Andre Noll [Mon, 23 Aug 2021 18:12:30 +0000 (20:12 +0200)]
mp4: Kill fix_byte_order_32().

All quantities stored in mp4 files are in big endian format, There's
no reason to "fix" anything, just write out the 32 bit numbers using

4 months agomp4: Avoid duplicating the list of atoms.
Andre Noll [Mon, 23 Aug 2021 14:37:33 +0000 (16:37 +0200)]
mp4: Avoid duplicating the list of atoms.

A little cpp magic can do wonders in this regard. The new
atom_name_to_type() should also be more efficient because we replaced
four 8-bit comparisons by one 32-bit comparison.

4 months agomp4: Merge parse_leaf_atom() into parse_sub_atoms().
Andre Noll [Mon, 23 Aug 2021 14:24:38 +0000 (16:24 +0200)]
mp4: Merge parse_leaf_atom() into parse_sub_atoms().

This gets rid of the distinction between atoms with and without
subatoms, which was confusing because some atoms "without" subatoms
in fact do contain subatoms, we just did not want to parse them
recursively in parse_sub_atoms().

With this weirdness gone, we may move on to simplify the atoms enum and
atom_name_to_type() further, but this is left to a subsequent patch.

4 months agomp4: Simplify parse_sub_atoms().
Andre Noll [Mon, 23 Aug 2021 14:18:17 +0000 (16:18 +0200)]
mp4: Simplify parse_sub_atoms().

This converts the while loop into a for loop and replaces the
counted_size variable by "dest" to clarify the loop structure. We
also move the two 8-bit variables into the loop as they are only used
there and skip their pointless initializations.

4 months agomp4: Use automatic numbering for atom enum.
Andre Noll [Sun, 22 Aug 2021 21:08:36 +0000 (23:08 +0200)]
mp4: Use automatic numbering for atom enum.

The exact numbers numbers of the ATOM enum are irrelevant. The only
thing which matters is the distinction between atoms we are only
interested in because they contain subatoms we care about and atoms
for which there is a corresponding read_xxx() parser.

4 months agomp4: Remove unused atoms.
Andre Noll [Sun, 22 Aug 2021 20:49:13 +0000 (22:49 +0200)]
mp4: Remove unused atoms.

The enum and atom_name_to_type() still knows about a lot of atoms we
don't care about. These only clutter up the code and slow things down,
so drop them.

4 months agomp4: Kill membuffer API.
Andre Noll [Sun, 22 Aug 2021 19:16:48 +0000 (21:16 +0200)]
mp4: Kill membuffer API.

Thanks to the previous cleanups, create_ilst() is the last remaining
membuffer user. Since the size of the ilst atom can be computed as the
sum of the tag lengths plus a constant times the number of tag items,
we can allocate a suitably sized buffer up-front instead of relying
on the membuffer framework to allocate and resize buffers as needed.

4 months agomp4: Assume udta, meta and ilst are always present.
Andre Noll [Sun, 22 Aug 2021 18:01:01 +0000 (20:01 +0200)]
mp4: Assume udta, meta and ilst are always present.

Under normal circumstances these atoms exist or can at least be
created by other means (e.g., by running mp4tags -a foo bar.m4a).

This patch makes mp4_open_meta() fail early if at least one of the
three atoms is missing. This allows to remove the (never tested hence
probably buggy) code which creates these atoms.

4 months agomp4: Merge membuffer_write_std_tag() into create_ilst().
Andre Noll [Sat, 21 Aug 2021 17:28:52 +0000 (19:28 +0200)]
mp4: Merge membuffer_write_std_tag() into create_ilst().

The former is short and is only called by the latter.

4 months agomp4: Eliminate duplication between the two open functions.
Andre Noll [Sat, 21 Aug 2021 15:56:17 +0000 (17:56 +0200)]
mp4: Eliminate duplication between the two open functions.

The only difference between mp4_open_read() and mp4_open_meta()
is that they pass different values for the meta_only flag to
parse_root_atoms(). We can avoid some duplication by moving the
common code to parse_root_atoms(). Rename that function to open_file()
because it now does more than just parsing atoms.

The patch also changes the prototype of both public open functions
to return an integer error code in addition to the pointer to an mp4
structure. This allows us to gradually improve the error diagnostics.

4 months agomp4: Remove find_atom() and find_atom_v2().
Andre Noll [Sat, 21 Aug 2021 14:23:05 +0000 (16:23 +0200)]
mp4: Remove find_atom() and find_atom_v2().

During mp4_open_meta() we encounter the ILST, META and UDTA atoms
but don't record the size and the location of these atoms. Doing
so allows us to use this information later in mp4_meta_update()
instead of calling find_atom() or find_atom_v2() to search the file
again. This removes some ugly code and speeds up the operation.

4 months agomp4: Get rid of find_standard_meta().
Andre Noll [Sat, 21 Aug 2021 11:30:02 +0000 (13:30 +0200)]
mp4: Get rid of find_standard_meta().

We don't need a dedicated function and data structure for that. Just
open-code the logic in create_ilst() and clean up this function a bit
while at it. Specifically:

* Call the loop variable "n" rather than "metaptr" since it is not
a pointer but an unsigned integer.

* Abort if we encounter a tag item name which is not one of the five
standard names. This can never occur because the origin of these
strings is the code in aac_afh.c which only passes standard names.

* Drop the integer return value, since the function can never
fail. Make it return the buffer pointer instead and get rid of the
corresponding parameter.

4 months agomp4: Improve parse_tag().
Andre Noll [Fri, 20 Aug 2021 12:23:04 +0000 (14:23 +0200)]
mp4: Improve parse_tag().

* Merge tag_add_field() and read_string() into parse_tag() since they
are simple enough and have only one caller.

* Avoid memory leaks in the error case.

* Let the function return an error code (rather than -1) in all cases,
and check the return value in the callers.

* Add a sanity check for the subsize.

* Avoid creating two copies of the tag value.

* Rename the variable for the tag value.

4 months agomp4: Simplify read_mp4a().
Andre Noll [Fri, 20 Aug 2021 11:38:55 +0000 (13:38 +0200)]
mp4: Simplify read_mp4a().

The single caller resets the file offset after the call, so we may
stop reading the atom after we've parsed the last field of interest,
which happens to be the sample rate.

4 months agomp4: Remove two local unused header_size variables.
Andre Noll [Fri, 20 Aug 2021 11:33:17 +0000 (13:33 +0200)]
mp4: Remove two local unused header_size variables.

The header size is an optional pointer argument of atom_read_header(),
i.e., callers may pass NULL if they aren't interested in the atom
header size.

4 months agomp4: Rename atom_read() to parse_leaf_atom().
Andre Noll [Thu, 19 Aug 2021 18:13:57 +0000 (20:13 +0200)]
mp4: Rename atom_read() to parse_leaf_atom().

For consistency and symmetry with parse_subatoms().

4 months agomp4: Reduce atom parsing to the bare minimum.
Andre Noll [Thu, 19 Aug 2021 18:07:13 +0000 (20:07 +0200)]
mp4: Reduce atom parsing to the bare minimum.

This replaces need_parse_when_meta_only() by need_atom() which is
called from parse_sub_atoms() for both regular opens and meta-only
opens to decide if the detected atom needs to be parsed.

After this patch we skip more atoms than we used to do, speeding up
the operation for both kinds of opens.

4 months agomp4: Convert "meta_only" to a boolean.
Andre Noll [Thu, 19 Aug 2021 17:13:31 +0000 (19:13 +0200)]
mp4: Convert "meta_only" to a boolean.

Several functions receive the "meta_only" parameter to distinguish
between regular and metadata-only opens. The parameter can only be
zero or one, so use a boolean because true/false is more descriptive
than 1/0.

4 months agomp4: Simplify parse_atoms().
Andre Noll [Thu, 19 Aug 2021 17:06:10 +0000 (19:06 +0200)]
mp4: Simplify parse_atoms().

We are only interested in subatoms of the moov atom, so skip everything
else. Rename the function to parse_root_atoms() and remove the comment
which does not convey any information anymore.

4 months agomp4: Remove two unused arrays from struct mp4_track.
Andre Noll [Thu, 19 Aug 2021 14:08:46 +0000 (16:08 +0200)]
mp4: Remove two unused arrays from struct mp4_track.

These arrays are allocated and initialized but their values are
never read.

4 months agomp4: Hide tracks array.
Andre Noll [Thu, 19 Aug 2021 13:52:13 +0000 (15:52 +0200)]
mp4: Hide tracks array.

All functions of mp4.c operate on the first audio track. This
patch makes this fact implicit which allows us to remove the public
mp4_get_total_tracks() and mp4_is_audio_track(). Moreover, the track
parameter can be removed from all public functions.

If no audio track was found in the mp4 file, we now return an error
from two public open functions of mp4.c. Otherwise, we maintain a
pointer to the first audio track within the mp4 structure and use
that to identify the track rather than letting the API users pass
the track number.

4 months agomp4: Simplify chunk_of_sample().
Andre Noll [Wed, 18 Aug 2021 21:15:07 +0000 (23:15 +0200)]
mp4: Simplify chunk_of_sample().

This function was unnecessarily complex. The equivalent replacement
code is much shorter and easier to read. Besides reducing the number
of local variables, we drop the chunk_sample parameter and return
this number via the return value of the function.

4 months agomp4: Merge chunk_to_offset() into mp4_set_sample_position().
Andre Noll [Wed, 18 Aug 2021 19:18:29 +0000 (21:18 +0200)]
mp4: Merge chunk_to_offset() into mp4_set_sample_position().

Another equivalent transformation which shortens the code and improves

4 months agomp4: Merge sample_range_size() into mp4_set_sample_position().
Andre Noll [Wed, 18 Aug 2021 19:08:45 +0000 (21:08 +0200)]
mp4: Merge sample_range_size() into mp4_set_sample_position().

This equivalent transformation shortens the code and improves

4 months agomp4: Provide return value for mp4_set_sample_position().
Andre Noll [Wed, 18 Aug 2021 18:59:25 +0000 (20:59 +0200)]
mp4: Provide return value for mp4_set_sample_position().

This function fails if the given parameters are invalid. Detect this
and return EINVAL in this case. Add corresponding error checking to
the aac audio format handler.

4 months agomp4: Simplify sample_range_size().
Andre Noll [Wed, 18 Aug 2021 18:46:52 +0000 (20:46 +0200)]
mp4: Simplify sample_range_size().

Reduce indentation by making the else branch unconditional and simply
call the track pointer "t" rather than "p_track".

4 months agomp4: Merge sample_to_offset() into mp4_set_sample_position().
Andre Noll [Wed, 18 Aug 2021 18:36:50 +0000 (20:36 +0200)]
mp4: Merge sample_to_offset() into mp4_set_sample_position().

The former is only called by the latter, and both are
short. De-obfuscate the code a little by avoiding pointless local

4 months agomp4: Rename mp4_total_tracks() to mp4_get_total_tracks().
Andre Noll [Wed, 18 Aug 2021 16:12:44 +0000 (18:12 +0200)]
mp4: Rename mp4_total_tracks() to mp4_get_total_tracks().

Just to be consistent with other public functions whose name contain
a predicate. Move the function down to related functions.

4 months agomp4: Remove ->error of struct mp4.
Andre Noll [Wed, 18 Aug 2021 16:08:38 +0000 (18:08 +0200)]
mp4: Remove ->error of struct mp4.

It's easier to have track_add(), the only function which sets ->error,
return an integer error code instead. Since track_add() is simple
and is only called by parse_sub_atoms(), open-code the logic there.

Also, don't reset ->total_tracks on errors because this leads to a
memory leak, don't increase the track counter on errors and remove
the comment which only states what is obvious.

4 months agomp4: Add error checking to parse_atoms() and friends.
Andre Noll [Wed, 18 Aug 2021 15:08:08 +0000 (17:08 +0200)]
mp4: Add error checking to parse_atoms() and friends.

After this patch read errors are propagated all the way down from the
read_data() primitive to the public entry functions mp4_open_read()
and mp4_open_meta().

4 months agomp4: Add error checking for atom_read().
Andre Noll [Wed, 18 Aug 2021 14:59:35 +0000 (16:59 +0200)]
mp4: Add error checking for atom_read().

While the individual atom parsers all perform error checking and
return an error code, their caller, atom_read(), ignores errors.

Address this shortcoming, simplify the function by using a switch
instead of an if-else chain and move the descriptions of the atoms
to the enum where they belong.

4 months agomp4: Improve handling of read errors.
Andre Noll [Wed, 18 Aug 2021 13:25:17 +0000 (15:25 +0200)]
mp4: Improve handling of read errors.

Currently read_data() of mp4.c is an atrocious mess. The ->read()
callback is defined to return uint32_t, but the return value is
stored in a signed 32 bit integer. Moreover, read_data() contains a
dead store, it handles neither short nor interrupted reads correctly,
and it moves the file position backwards on errors.

While this is easy to fix, a more intricate problem is that most
callers of read_data(), including all read_intX() helpers, ignore the
return value of read_data() and return uninitialized stack contents in
the error case. This is kind of dealt with by the ->read_error member
of struct mp4, but this not more than a kludge, which, according to
the comments, was applied after several CVEs had been filed against
the library.

Let's DTRT here, even though it adds a fair amount of new code:
Check the return value of each read operation and fail early on errors.

We have to distinguish three cases: error, EOF, and success, encoded
as return values -1, 0 and 1, respectively. This commit converts most
functions which read from an mp4 file to this convention. More work
is required as return values are not checked everywhere yet. This was
left for subsequent commits to keep the already large patch within
reasonable size.

Since we don't rely on ->read_error of struct mp4 any more, it can
be removed.

4 months agomp4: Remove dead store from find_atom_v2().
Andre Noll [Wed, 18 Aug 2021 13:15:41 +0000 (15:15 +0200)]
mp4: Remove dead store from find_atom_v2().

The "size" variable is not used in the code which follows the loop
we are breaking out here.

4 months agomp4: Move read_intX functions.
Andre Noll [Wed, 18 Aug 2021 13:11:09 +0000 (15:11 +0200)]
mp4: Move read_intX functions.

This way, they are located next to each other, and are ordered by the
size of the integer value they read.

Pure code movement, no real changes.

4 months agomp4: Rename read_char() to read_int8().
Andre Noll [Wed, 18 Aug 2021 13:08:25 +0000 (15:08 +0200)]
mp4: Rename read_char() to read_int8().

That's more to the point, and is consistent with the other functions
which read integer values.

4 months agomp4: Make channel count a 16 bit quantity.
Andre Noll [Wed, 18 Aug 2021 13:05:09 +0000 (15:05 +0200)]
mp4: Make channel count a 16 bit quantity.

It gets initialized by reading an 16 bit integer.

4 months agomp4: Make most members of struct mp4_track unsigned.
Andre Noll [Wed, 18 Aug 2021 13:02:43 +0000 (15:02 +0200)]
mp4: Make most members of struct mp4_track unsigned.

All of these get initialized by reading an unsigned 32 bit value from
the file, so they should also be unsigned.

4 months agomp4: Merge membuffer_free() into membuffer_transfer_from_file().
Andre Noll [Sat, 14 Aug 2021 21:20:15 +0000 (23:20 +0200)]
mp4: Merge membuffer_free() into membuffer_transfer_from_file().

The latter is the only caller of the former.

4 months agomp4: Clean up membuffer_transfer_from_file().
Andre Noll [Sat, 14 Aug 2021 21:14:38 +0000 (23:14 +0200)]
mp4: Clean up membuffer_transfer_from_file().

The buffer pointer can never be NULL, so drop this check. Next, instead
of defining a void * pointer and cast it to char *, use char * directly.
Finally, the cast to unsigned has no effect, so drop it.

4 months agomp4: Drop return value from membuffer_write() and friends.
Andre Noll [Sat, 14 Aug 2021 21:07:11 +0000 (23:07 +0200)]
mp4: Drop return value from membuffer_write() and friends.

This function always returns the value it received via the last
argument. Pointless.

4 months agomp4: Remove the membuffer error bit.
Andre Noll [Sat, 14 Aug 2021 20:53:39 +0000 (22:53 +0200)]
mp4: Remove the membuffer error bit.

Due to the previous cleanups, it is never set, so remove all code
which checks whether the bit is set.

4 months agomp4: Free the membuffer in membuffer_detach().
Andre Noll [Sat, 14 Aug 2021 20:46:50 +0000 (22:46 +0200)]
mp4: Free the membuffer in membuffer_detach().

Each call to this function is followed by a call to membuffer_free(),
which frees the membuffer but not the data buffer because that was
set to NULL.

It is simpler to free the membuffer directly in membuffer_detach().