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.
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.
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.
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.
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.
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.
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.
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().
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.
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.
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.
Andre Noll [Sat, 14 Aug 2021 20:40:00 +0000 (22:40 +0200)]
mp4: Simplify membuffer_create().
Since para_malloc() never returns NULL, the error state can only be
zero. Use para_calloc(), skip the zero initializations and kill a
pointless local variable.
Andre Noll [Sat, 14 Aug 2021 20:27:12 +0000 (22:27 +0200)]
mp4: Check return value of membuffer_transfer_from_file().
This function calls the ->read() method of the callback, which may
fail. Currently all three callers ignore the return value and rely
on the fact that the membuffer is set to error state, which will be
detected later.
It's easier and clearer to check for errors in the callers and fail
early on read errors. Since the membuffer is useless in the error
case, free it right away in membuffer_transfer_from_file(). Change
the function to return bool instead of unsigned while at it and remove
a pointless cast in one of its callers.
Andre Noll [Sat, 14 Aug 2021 18:53:15 +0000 (20:53 +0200)]
mp4: Drop integer return type from modify_moov().
This function returns either zero or one to indicate success. On
success, a pointer to a buffer and the buffer size are returned. It
is simpler and less redundant to indicate failure by returning a NULL
buffer pointer. Rather than using a void ** argument for the buffer,
let the function return void *.
Andre Noll [Sat, 14 Aug 2021 18:21:51 +0000 (20:21 +0200)]
mp4: Merge mp4_close() and tag_delete().
The latter is only called by the former, and both are short enough.
Don't bother to zero out meta->tags and meta->count because we free
the containing mp4 structure as well.
Andre Noll [Sat, 14 Aug 2021 17:38:21 +0000 (19:38 +0200)]
mp4: Clean up membuffer_write_std_tag().
Remove the check for the compilation flag since we never pass "cpil"
to this function. Remove the flags variable whose value is now always
one. Introduce a variable for the string length instead of calling
strlen() three times, and unify the way comments are formatted.
Andre Noll [Sat, 14 Aug 2021 17:22:17 +0000 (19:22 +0200)]
mp4: Clean up find_standard_meta().
Use ARRAY_SIZE() instead of open-coding it, move the stdmetas array
into the function since it is only used there, and make it const.
Also replace 0 by NULL, since the function returns a pointer, and
remove the pointless comment.
Finally, move the function and the declaration of the stdmeta_entry
structure closer to its single user.
Andre Noll [Sat, 14 Aug 2021 17:15:29 +0000 (19:15 +0200)]
mp4: Simplify create_ilst().
This function contains a lot of overhead which is just dead code
for paraslash since we only care about five standard tags, In
particular, we never write custom tags. Removing the single caller
of membuffer_write_custom_tag() left a whole bunch of other functions
and data structures unused, so these can be removed as well.
Andre Noll [Sat, 14 Aug 2021 17:00:06 +0000 (19:00 +0200)]
mp4: Call metadata structures "meta".
Currently they are called "tags" or "data", both of which are confusing
because struct mp4_metadata has a member called "tags", and "data"
is also used for generic buffers in the various I/O helpers.
Andre Noll [Sat, 14 Aug 2021 15:53:59 +0000 (17:53 +0200)]
mp4: Simplify and speed up metadata editing.
Currently the aac audio format handler first calls mp4_open_meta()
to get the metadata tags, then alters the in-memory structure of the
tags according to the command line options and passes this modified
structure to mp4_meta_update() to rewrite the tags. This latter call
parses the tags again, which is unnecessary overhead.
This patch changes the signature of mp4_meta_update() to accept an
mp4 structure instead of a callback structure and uses that instead
of re-opening the file.
Andre Noll [Fri, 13 Aug 2021 18:51:38 +0000 (20:51 +0200)]
mp4: Simplify and doxify meta tag accessors.
The integer return value is redundant, so get rid of the value
parameter and simplify meta_find_by_name() accordingly. Document that
tag values are allocated on the heap and should be freed by the caller.
Andre Noll [Fri, 13 Aug 2021 18:19:43 +0000 (20:19 +0200)]
mp4: Simplify parse_tag().
We don't care about arbitrarily named tags, and those tags we're
interested in are generally present in form of the standard tags
(ATOM_TITLE, ATOM_ARTIST etc.).
Since we now always call get_metadata_name() to get the string
representation of the tag, we don't need to make a copy any more,
just pass the const pointer directly to tag_add_field().
With this change in place it is obvious that we never pass a NULL or
empty tag name to tag_add_field(), and we don't pass a NULL pointer
for the value argument either, so remove the safety check.
Andre Noll [Wed, 11 Aug 2021 20:52:11 +0000 (22:52 +0200)]
mp4: Rename and simplify set_metadata_name()
This function is an atrocious mess. For one, the naming is confusing
because the function does not set the atom name, it *returns* it.
More importantly, the function defines a static array for no good
reason and then hard-codes the array indices in a large switch
statement.
It's much easier to simply return a pointer to a string literal and
perform the strdup operation in the single caller.
Andre Noll [Wed, 11 Aug 2021 19:45:12 +0000 (21:45 +0200)]
mp4: Use uniform names for callback and mp4 structures.
Currently it's a confusing mess, with callbacks called f, ff, or
stream, where the former two are also used for pointers to struct
mp4. Let's call a spade a spade and use cb everywhere for the callbacks
while f is reserved to denote an mp4 pointer.
Andre Noll [Wed, 11 Aug 2021 19:37:16 +0000 (21:37 +0200)]
mp4: Hide ->read_error.
This does not belong into the callback structure whose fields are
supposed to get initialized by the audio format handler. Move it to
the internal struct mp4 instead, next to the existing error counter.
Andre Noll [Wed, 11 Aug 2021 17:26:00 +0000 (19:26 +0200)]
mp4: Don't parse the esds atom any more.
With the decoder specific config no longer in use, we can simplify
mp4.c further by getting rid of some cryptic and underdocumented code
which no longer does anything useful for us.
Andre Noll [Wed, 11 Aug 2021 17:12:07 +0000 (19:12 +0200)]
mp4: Introduce mp4_is_audio_track().
Currently the aac audio format handler iterates over the tracks
in an mp4 file. For each track it tries to get the audio-specific
configuration by calling mp4_get_decoder_config() and calls into faad
to check whether it is a valid configuration for the aac decoder.
We can simplify all this because the mp4 code already knows the type
of each track, albeit it does not expose this information yet. So
provide the new mp4_is_audio_track() helper and let the aac audio
format handler pick the first track for which this helper returns true.
As an additional benefit, we can remove the now unused
mp4_get_decoder_config().
The old name is misleading because the returned file handle is by no
means opened read-only. In fact we call mp4ff_meta_update() on it,
which alters the file to store the modified metadata.
Andre Noll [Wed, 11 Aug 2021 13:41:37 +0000 (15:41 +0200)]
mp4: Introduce mp4ff_get_duration().
This allows us to get rid of an ugly hack in aac_afh.c where we peeked
at the audio-specific config structure to get the scaling factor which
was needed to compute the duration.
Andre Noll [Tue, 10 Aug 2021 16:57:42 +0000 (18:57 +0200)]
mp4: Remove const qualifier from non-pointer function arguments.
In contrast to the pointer case, it's generally not very interesting
to know whether a function will modify the automatic variable which
corresponds to a non-pointer argument.
Andre Noll [Mon, 9 Aug 2021 22:14:53 +0000 (00:14 +0200)]
mp4: Reduce indentation of create_ilst().
This function contained an indented block for no good reason. No real
change except that a few variables are exposed to the code below the
former block, but this is not a problem.
Andre Noll [Mon, 9 Aug 2021 21:13:35 +0000 (23:13 +0200)]
mp4: Use read/write functions from portable_io.h.
This removes quite a bit of ugly code. In particular, atom_get_size()
is completely unnecessary and can be removed. Since there is no
function in portable_io.h to read a 24 bit integer, we have to add
read_u24_be().
Andre Noll [Mon, 9 Aug 2021 18:27:28 +0000 (20:27 +0200)]
mp4: Further reduce the indentation of mp4ff_parse_tag().
Handle the easy cases first. Again, this patch looks unreviewable,
but it in essence it is only one large block that is shifted to the
left with no other changes.
Andre Noll [Mon, 9 Aug 2021 18:05:10 +0000 (20:05 +0200)]
mp4: Use para_strdup().
Like malloc(), strdup() may fail. The current code does not always
check that. Using the paraslash wrapper avoids the issue and has the
additional benefit that para_strdup(NULL) does not segfault.
Andre Noll [Mon, 9 Aug 2021 17:43:36 +0000 (19:43 +0200)]
mp4: Use para_{c,m}alloc() to allocate memory.
With mp4ff being internal we can afford to just abort if allocations
fail (which is what the para_ wrappers do). This allows us to simplify
the code quite a bit.
The code used to cast the pointer returned from malloc(). This is
unnecessary, so drop these casts.
Andre Noll [Mon, 9 Aug 2021 17:10:46 +0000 (19:10 +0200)]
mp4: Simplify and rename mp4ff_read_sample_getsize().
The new code is much shorter. It is equivalent because the single
caller only checks whether the return value is less or equal then zero,
and does not use the return value in this case.
Andre Noll [Mon, 9 Aug 2021 16:43:44 +0000 (18:43 +0200)]
aac: Move declaration of mp4ff_set_sample_position() to mp4.h.
We used to provide the declaration of mp4ff_set_sample_position()
in aac_afh.c. With the internal mp4.c this is no longer necessary as
we may as well declare the function in mp4.h.
Remove the documentation since it has become stale.
Andre Noll [Sun, 8 Aug 2021 20:09:28 +0000 (22:09 +0200)]
mp4: Make non-public functions static.
It's generally a good thing to reduce the scope to the compilation
unit where possible. Static functions also reduce the number of
symbols and help the compiler to optimize the generated code.
Andre Noll [Thu, 21 Oct 2021 17:09:59 +0000 (19:09 +0200)]
mp4: Wire it in and make it compile cleanly.
This switches to our local copy of libmp4ff. The configure script
now turns on support for the aac audio format even if libmp4ff is
not installed.
Some changes to mp4.c were necessary to silence warnings, most of
which are trivial. The single notably non-trivial change is the missing
initialization of chunk_sample and chunk in mp4ff_chunk_of_sample().
Andre Noll [Sun, 8 Aug 2021 19:13:41 +0000 (21:13 +0200)]
Provide our own copy of libmp4ff.
This library was removed from the faad repository in 2017. This
commit put everything we need from the libmp4ff code base into the
new mp4.c and mp4.h files. All content was taken unmodified from
commit 1cf5a505daf0 of the faad2 repo, the last commit that contains
the library.
The code needs a lot of cleanup and does not compile without warnings.
We commit it as-is to track what exactly has been copied from
libmp4ff. The new mp4.c has not been included into the build system,
however, so this commit has no effect yet.
It is broken because the recipe for the .d files does not honor
CPPFLAGS since we only modify the CPPFLAGS variable for the .o
targets. Adding the corresponding .d targets would be too ugly,
so revert the commit for now until a better solution has been found.
Andre Noll [Mon, 14 Mar 2022 21:28:27 +0000 (22:28 +0100)]
Makefile: Fix compilation after header removal.
When switching from an older git version which still contains some
header file to a newer version where it got removed, a .d file remains
in build/deps which lists the no longer existing header file as a
prerequisite. This causes the build to fail because the prerequisite
cannot be created. The problem can be worked around by removing the
stale .d file, for example by running make clean, but this is no real
fix, and is inefficient.
The root of the matter is that .d files depend on their .c counterpart,
but this dependency is not stated anywhere in the Makefile. Thus, we
need a rule for the .d target with the same prerequisites and the same
recipe as the object file target. GNU make supports multiple targets,
but the feature does not seem to work as advertised, regardless of
whether rules with independent targets or rules with grouped targets
(using the &: separator) are employed. Thus we bite the bullet and
use two separate rules.