[FFmpeg-devel] [PATCH v7 1/2] avformat/imf: Demuxer

[Lynne]

6 Dec 2021, 04:18 by pal at sandflow.com:

> From: Pierre-Anthony Lemieux <pal at palemieux.com>
> +
> +/**
> + * Implements IMP CPL processing
> + *
> + * @author Pierre-Anthony Lemieux
> + * @file
> + * @ingroup lavu_imf
> + */
> +
> +#include "imf.h"
> +#include "libavformat/mxf.h"
> +#include "libavutil/bprint.h"
> +#include "libavutil/error.h"
> +#include <libxml/parser.h>
> +
> +xmlNodePtr ff_xml_get_child_element_by_name(xmlNodePtr parent, const char *name_utf8)
> +{
> +    xmlNodePtr cur_element;
> +
> +    cur_element = xmlFirstElementChild(parent);
> +    while (cur_element) {
> +        if (xmlStrcmp(cur_element->name, name_utf8) == 0)
> +            return cur_element;
> +        cur_element = xmlNextElementSibling(cur_element);
> +    }
> +    return NULL;
> +}
> +
> +int ff_xml_read_UUID(xmlNodePtr element, uint8_t uuid[16])
> +{
> +    xmlChar *element_text = NULL;
> +    int scanf_ret;
> +    int ret = 0;
> +
> +    element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1);
> +    scanf_ret = sscanf(element_text,
> +        FF_UUID_FORMAT,
> +        &uuid[0],
> +        &uuid[1],
> +        &uuid[2],
> +        &uuid[3],
> +        &uuid[4],
> +        &uuid[5],
> +        &uuid[6],
> +        &uuid[7],
> +        &uuid[8],
> +        &uuid[9],
> +        &uuid[10],
> +        &uuid[11],
> +        &uuid[12],
> +        &uuid[13],
> +        &uuid[14],
> +        &uuid[15]);
> +    if (scanf_ret != 16) {
> +        av_log(NULL, AV_LOG_ERROR, "Invalid UUID\n");
> +        ret = AVERROR_INVALIDDATA;
> +    }
> +    xmlFree(element_text);
> +
> +    return ret;
> +}
>

Please don't NIH-parse UUIDs. They're more complicated
than this. Use libuuid, it's stable, bug-free and universally
available. I submitted a patch for it last month for an
unrelated use, which I decided to drop, but you
can see how it was done there.

Also, please follow the code style. We use snake_case
for function names, which means there are no capital
letters in them.
There are also still one or two one-line statements
left that are wrapped in brackets.

We check every single allocation we make, since malloc()
can fail on Windows. So that means all av_strdup calls,
av_append_path_component calls and so on can
return NULL, and result in a nasty crash.

Do not use strtok. It's not thread-safe at all. Use av_strtok,
which is thread-safe.

ff_imf_cpl_free() has a weird branch that can just be
done with a return if cpl is NULL.

There are places where you call av_free() and then
set the pointer to NULL. We have a convenience
function for this, av_freep().

Please use defined-length integers instead of "unsigned long".
If something needs to be 64-bits, use (u)int64_t. If something
doesn't need to be 64-bits, use uint32_t for unsigned, or plain
int for signed. We always require plain ints to be 32-bits.

Also, instead of printing 64-bit ints via "%lu", use "%"PRIu64"
and "%"PRIi64". MSVC has problems with %lu.