[FFmpeg-devel,v2,1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122

Co-authored-by: Pierre-Anthony Lemieux <pal@palemieux.com>
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavutil/Makefile |   2 +
 libavutil/uuid.c   | 155 +++++++++++++++++++++++++++++++++++++++++++++
 libavutil/uuid.h   | 137 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 294 insertions(+)
 create mode 100644 libavutil/uuid.c
 create mode 100644 libavutil/uuid.h

Zane van Iperen:
> Co-authored-by: Pierre-Anthony Lemieux <pal@palemieux.com>
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  libavutil/Makefile |   2 +
>  libavutil/uuid.c   | 155 +++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/uuid.h   | 137 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 294 insertions(+)
>  create mode 100644 libavutil/uuid.c
>  create mode 100644 libavutil/uuid.h
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 81df3b0640..29742668b8 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -82,6 +82,7 @@ HEADERS = adler32.h                                                     \
>            timestamp.h                                                   \
>            tree.h                                                        \
>            twofish.h                                                     \
> +          uuid.h                                                        \
>            version.h                                                     \
>            video_enc_params.h                                            \
>            xtea.h                                                        \
> @@ -174,6 +175,7 @@ OBJS = adler32.o                                                        \
>         tx_float.o                                                       \
>         tx_double.o                                                      \
>         tx_int32.o                                                       \
> +       uuid.o                                                           \
>         video_enc_params.o                                               \
>         film_grain_params.o                                              \
>  
> diff --git a/libavutil/uuid.c b/libavutil/uuid.c
> new file mode 100644
> index 0000000000..6cf7a20c23
> --- /dev/null
> +++ b/libavutil/uuid.c
> @@ -0,0 +1,155 @@
> +/*
> + * Copyright (c) 2022 Pierre-Anthony Lemieux <pal@palemieux.com>
> + *                    Zane van Iperen <zane@zanevaniperen.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/*
> + * Copyright (C) 1996, 1997 Theodore Ts'o.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, and the entire permission notice in its entirety,
> + *    including the disclaimer of warranties.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote
> + *    products derived from this software without specific prior
> + *    written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
> + * WHICH ARE HEREBY DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
> + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> + * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
> + * DAMAGE.
> + */
> +
> +/**
> + * @file
> + * UUID parsing and serialization utilities.
> + * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
> + * i.e. ignoring the internal layout of the UUID, which depends on the type
> + * of the UUID.
> + *
> + * @author Pierre-Anthony Lemieux <pal@palemieux.com>
> + * @author Zane van Iperen <zane@zanevaniperen.com>
> + */
> +
> +#include "uuid.h"
> +#include "error.h"
> +#include "avstring.h"
> +#include <stdlib.h>
> +#include <string.h>
> +
> +int av_uuid_parse(const char *in, AVUUID uu)
> +{
> +    if (strlen(in) != 36)
> +        return AVERROR(EINVAL);
> +
> +    return av_uuid_parse_range(in, in + 36, uu);
> +}
> +
> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu)
> +{
> +    int i;
> +    const char *cp;
> +    char buf[3];
> +
> +    if ((in_end - in_start) != 36)
> +        return -1;
> +
> +    for (i = 0, cp = in_start; i < 36; i++, cp++) {
> +        if (i == 8 || i == 13 || i == 18 || i == 23) {
> +            if (*cp == '-')
> +                continue;
> +            return AVERROR(EINVAL);
> +        }
> +
> +        if (!av_isxdigit(*cp))
> +            return AVERROR(EINVAL);
> +    }
> +
> +    buf[2] = '\0';
> +    for (i = 0, cp = in_start; i < 16; i++) {
> +        if (i == 4 || i == 6 || i == 8 || i == 10)
> +            cp++;
> +
> +        buf[0] = *cp++;
> +        buf[1] = *cp++;
> +
> +        errno = 0;
> +        uu[i] = strtoul(buf, NULL, 16);
> +        if (errno)
> +            return AVERROR(errno);

How could this ever happen given that you have already checked that the
buffer only contains hex digits? And isn't using strtoul a bit overkill
anyway? I'd just check and parse this stuff in one loop.

> +    }
> +
> +    return 0;
> +}
> +
> +static char const hexdigits_lower[16] = "0123456789abcdef";
> +
> +void av_uuid_unparse(const AVUUID uuid, char *out)
> +{
> +    char *p = out;
> +
> +    for (int i = 0; i < 16; i++) {
> +        uint8_t tmp;
> +
> +        if (i == 4 || i == 6 || i == 8 || i == 10)
> +            *p++ = '-';
> +
> +        tmp = uuid[i];
> +        *p++ = hexdigits_lower[tmp >> 4];
> +        *p++ = hexdigits_lower[tmp & 15];
> +    }
> +
> +    *p = '\0';
> +}
> +
> +int av_uuid_urn_parse(const char *in, AVUUID uu)
> +{
> +    if (av_stristr(in, "urn:uuid:") != in)
> +        return AVERROR(EINVAL);
> +
> +    return av_uuid_parse(in + 9, uu);
> +}
> +
> +int av_uuid_equal(const AVUUID uu1, const AVUUID uu2)
> +{
> +    return memcmp(uu1, uu2, AV_UUID_LEN) == 0;
> +}
> +
> +void av_uuid_copy(AVUUID dest, const AVUUID src)
> +{
> +    memcpy(dest, src, AV_UUID_LEN);
> +}
> +
> +void av_uuid_nil_set(AVUUID uu)
> +{
> +    memset(uu, 0, AV_UUID_LEN);
> +}
> diff --git a/libavutil/uuid.h b/libavutil/uuid.h
> new file mode 100644
> index 0000000000..d7cd382ec2
> --- /dev/null
> +++ b/libavutil/uuid.h
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright (c) 2022 Pierre-Anthony Lemieux <pal@palemieux.com>
> + *                    Zane van Iperen <zane@zanevaniperen.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * UUID parsing and serialization utilities.
> + * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
                       ^
                       s

> + * i.e. ignoring the internal layout of the UUID, which depends on the type
> + * of the UUID.
> + *
> + * @author Pierre-Anthony Lemieux <pal@palemieux.com>
> + * @author Zane van Iperen <zane@zanevaniperen.com>
> + */
> +
> +#ifndef AVUTIL_UUID_H
> +#define AVUTIL_UUID_H
> +
> +#include <stdint.h>
> +
> +#define AV_PRI_UUID                          \
> +    "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-" \
> +    "%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> +
> +#define AV_PRI_URN_UUID                               \
> +    "urn:uuid:%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-" \
> +    "%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> +
> +/* AV_UUID_ARG() is used together with AV_PRI_UUID() or AV_PRI_URN_UUID
> + * to print UUIDs, e.g.
> + * av_log(NULL, AV_LOG_DEBUG, "UUID: " AV_PRI_UUID, AV_UUID_ARG(uuid));
> + */
> +#define AV_UUID_ARG(x)                  \
> +    (x)[ 0], (x)[ 1], (x)[ 2], (x)[ 3], \
> +    (x)[ 4], (x)[ 5], (x)[ 6], (x)[ 7], \
> +    (x)[ 8], (x)[ 9], (x)[10], (x)[11], \
> +    (x)[12], (x)[13], (x)[14], (x)[15]
> +
> +#define AV_UUID_LEN 16
> +
> +/* Binary representation of a UUID */
> +typedef uint8_t AVUUID[AV_UUID_LEN];
> +
> +/**
> + * Parses a string representation of a UUID formatted according to IETF RFC 4122
> + * into an AVUUID. The parsing is case-insensitive. The string must be 37
> + * characters long, including the terminating NULL character.

NUL, NULL is for pointers.

> + *
> + * Example string representation: "2fceebd0-7017-433d-bafb-d073a7116696"
> + *
> + * @param[in]  in  String representation of a UUID,
> + *                 e.g. 2fceebd0-7017-433d-bafb-d073a7116696
> + * @param[out] uu  AVUUID
> + * @return         A non-zero value in case of an error.
> + */
> +int av_uuid_parse(const char *in, AVUUID uu);
> +
> +/**
> + * Parses a URN representation of a UUID, as specified at IETF RFC 4122,
> + * into an AVUUID. The parsing is case-insensitive. The string must be 46
> + * characters long, including the terminating NULL character.
> + *
> + * Example string representation: "urn:uuid:2fceebd0-7017-433d-bafb-d073a7116696"
> + *
> + * @param[in]  in  URN UUID
> + * @param[out] uu  AVUUID
> + * @return         A non-zero value in case of an error.
> + */
> +int av_uuid_urn_parse(const char *in, AVUUID uu);
> +
> +/**
> + * Parses a string representation of a UUID formatted according to IETF RFC 4122
> + * into an AVUUID. The parsing is case-insensitive. The string must consist of
> + * 36 characters, i.e. `in_end - in_start == 36`
> + *
> + * @param[in]  in_start Pointer to the first character of the string representation
> + * @param[in]  in_end   Pointer to the character after the last character of the
> + *                      string representation. That memory location is never
> + *                      accessed
> + * @param[out] uu       AVUUID
> + * @return              A non-zero value in case of an error.
> + */
> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu);

This sounds like in_end is actually redundant. Does retaining it improve
extensibility?

> +
> +/**
> + * Serializes a AVUUID into a string representation according to IETF RFC 4122.
> + * The string is lowercase and always 37 characters long, including the
> + * terminating NULL character.
> + *
> + * @param[in]  uu  AVUUID
> + * @param[out] out Pointer to a array of no less than 37 characters.
                                  ^
                                  n

> + * @return         A non-zero value in case of an error.
> + */
> +void av_uuid_unparse(const AVUUID uu, char *out);
> +
> +/**
> + * Compares two UUIDs for equality.
> + *
> + * @param[in]  uu1  AVUUID
> + * @param[in]  uu2  AVUUID
> + * @return          Nonzero if uu1 and uu2 are identical, 0 otherwise
> + */
> +int av_uuid_equal(const AVUUID uu1, const AVUUID uu2);
> +
> +/**
> + * Copies the bytes of src into dest.
> + *
> + * @param[out]  dest  AVUUID
> + * @param[in]   src   AVUUID
> + */
> +void av_uuid_copy(AVUUID dest, const AVUUID src);
> +
> +/**
> + * Sets a UUID to nil
> + *
> + * @param[in,out]  uu  UUID to be set to nil
> + */
> +void av_uuid_nil_set(AVUUID uu);

Why are these three functions not static inline? Exporting them seems
like a waste.

> +
> +#endif /* AVUTIL_UUID_H */

Quoting Zane van Iperen (2022-04-24 12:14:03)
> +void av_uuid_nil_set(AVUUID uu)
                ^^^^^^^
sounds weird

av_uuid_zero()?
av_uuid_reset()?

On 10/5/22 23:18, Andreas Rheinhardt wrote:

>> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu)
>> +{
>> +    int i;
>> +    const char *cp;
>> +    char buf[3];
>> +
>> +    if ((in_end - in_start) != 36)
>> +        return -1;
>> +
>> +    for (i = 0, cp = in_start; i < 36; i++, cp++) {
>> +        if (i == 8 || i == 13 || i == 18 || i == 23) {
>> +            if (*cp == '-')
>> +                continue;
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        if (!av_isxdigit(*cp))
>> +            return AVERROR(EINVAL);
>> +    }
>> +
>> +    buf[2] = '\0';
>> +    for (i = 0, cp = in_start; i < 16; i++) {
>> +        if (i == 4 || i == 6 || i == 8 || i == 10)
>> +            cp++;
>> +
>> +        buf[0] = *cp++;
>> +        buf[1] = *cp++;
>> +
>> +        errno = 0;
>> +        uu[i] = strtoul(buf, NULL, 16);
>> +        if (errno)
>> +            return AVERROR(errno);
> 
> How could this ever happen given that you have already checked that the
> buffer only contains hex digits? And isn't using strtoul a bit overkill
> anyway? I'd just check and parse this stuff in one loop.
> 

Yeah, good point. It's based off libuuid's, which has some time/clock uuid handling
between the two loops. I'll tidy it up in the next few days... hopefully...

>> +/**
>> + * @file
>> + * UUID parsing and serialization utilities.
>> + * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
>                         ^
>                         s

Fixed.

>> +/**
>> + * Parses a string representation of a UUID formatted according to IETF RFC 4122
>> + * into an AVUUID. The parsing is case-insensitive. The string must be 37
>> + * characters long, including the terminating NULL character.
> 
> NUL, NULL is for pointers.
> 

Changed.

>> +/**
>> + * Parses a string representation of a UUID formatted according to IETF RFC 4122
>> + * into an AVUUID. The parsing is case-insensitive. The string must consist of
>> + * 36 characters, i.e. `in_end - in_start == 36`
>> + *
>> + * @param[in]  in_start Pointer to the first character of the string representation
>> + * @param[in]  in_end   Pointer to the character after the last character of the
>> + *                      string representation. That memory location is never
>> + *                      accessed
>> + * @param[out] uu       AVUUID
>> + * @return              A non-zero value in case of an error.
>> + */
>> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu);
> 
> This sounds like in_end is actually redundant. Does retaining it improve
> extensibility?
> 

I believe so. The main difference is av_uuid_parse_range() can handle non NUL-termiated
strings. I can just remove the entire last sentence (or change "must" to "should").

>> +
>> +/**
>> + * Serializes a AVUUID into a string representation according to IETF RFC 4122.
>> + * The string is lowercase and always 37 characters long, including the
>> + * terminating NULL character.
>> + *
>> + * @param[in]  uu  AVUUID
>> + * @param[out] out Pointer to a array of no less than 37 characters.
>                                    ^
>                                    n
> 

Fixed.


>> +/**
>> + * Sets a UUID to nil
>> + *
>> + * @param[in,out]  uu  UUID to be set to nil
>> + */
>> +void av_uuid_nil_set(AVUUID uu);
> 
> Why are these three functions not static inline? Exporting them seems
> like a waste.
> 

No particular reason, it's easy enough to change.

On 11/5/22 21:53, Anton Khirnov wrote:
> Quoting Zane van Iperen (2022-04-24 12:14:03)
>> +void av_uuid_nil_set(AVUUID uu)
>                  ^^^^^^^
> sounds weird
> 
> av_uuid_zero()?
> av_uuid_reset()?
> 

Good point, `av_uuid_zero()` has a nice ring to it.

On Wed, May 11, 2022 at 8:03 AM Zane van Iperen <zane@zanevaniperen.com> wrote:
>
>
>
> On 11/5/22 21:53, Anton Khirnov wrote:
> > Quoting Zane van Iperen (2022-04-24 12:14:03)
> >> +void av_uuid_nil_set(AVUUID uu)
> >                  ^^^^^^^
> > sounds weird
> >
> > av_uuid_zero()?
> > av_uuid_reset()?
> >
>
> Good point, `av_uuid_zero()` has a nice ring to it.

I think we should keep `nil` in the name since the _nil UUID_ has a
special meaning in RFC 4122. have proposed av_uuid_nil() in v3 of the
patchset.


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

On Wed, May 11, 2022 at 8:02 AM Zane van Iperen <zane@zanevaniperen.com> wrote:
>
>
>
> On 10/5/22 23:18, Andreas Rheinhardt wrote:
>
> >> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu)
> >> +{
> >> +    int i;
> >> +    const char *cp;
> >> +    char buf[3];
> >> +
> >> +    if ((in_end - in_start) != 36)
> >> +        return -1;
> >> +
> >> +    for (i = 0, cp = in_start; i < 36; i++, cp++) {
> >> +        if (i == 8 || i == 13 || i == 18 || i == 23) {
> >> +            if (*cp == '-')
> >> +                continue;
> >> +            return AVERROR(EINVAL);
> >> +        }
> >> +
> >> +        if (!av_isxdigit(*cp))
> >> +            return AVERROR(EINVAL);
> >> +    }
> >> +
> >> +    buf[2] = '\0';
> >> +    for (i = 0, cp = in_start; i < 16; i++) {
> >> +        if (i == 4 || i == 6 || i == 8 || i == 10)
> >> +            cp++;
> >> +
> >> +        buf[0] = *cp++;
> >> +        buf[1] = *cp++;
> >> +
> >> +        errno = 0;
> >> +        uu[i] = strtoul(buf, NULL, 16);
> >> +        if (errno)
> >> +            return AVERROR(errno);
> >
> > How could this ever happen given that you have already checked that the
> > buffer only contains hex digits? And isn't using strtoul a bit overkill
> > anyway? I'd just check and parse this stuff in one loop.
> >
>
> Yeah, good point. It's based off libuuid's, which has some time/clock uuid handling
> between the two loops. I'll tidy it up in the next few days... hopefully...
>
> >> +/**
> >> + * @file
> >> + * UUID parsing and serialization utilities.
> >> + * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
> >                         ^
> >                         s
>
> Fixed.
>
> >> +/**
> >> + * Parses a string representation of a UUID formatted according to IETF RFC 4122
> >> + * into an AVUUID. The parsing is case-insensitive. The string must be 37
> >> + * characters long, including the terminating NULL character.
> >
> > NUL, NULL is for pointers.
> >
>
> Changed.
>
> >> +/**
> >> + * Parses a string representation of a UUID formatted according to IETF RFC 4122
> >> + * into an AVUUID. The parsing is case-insensitive. The string must consist of
> >> + * 36 characters, i.e. `in_end - in_start == 36`
> >> + *
> >> + * @param[in]  in_start Pointer to the first character of the string representation
> >> + * @param[in]  in_end   Pointer to the character after the last character of the
> >> + *                      string representation. That memory location is never
> >> + *                      accessed
> >> + * @param[out] uu       AVUUID
> >> + * @return              A non-zero value in case of an error.
> >> + */
> >> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu);
> >
> > This sounds like in_end is actually redundant. Does retaining it improve
> > extensibility?
> >
>
> I believe so. The main difference is av_uuid_parse_range() can handle non NUL-termiated
> strings. I can just remove the entire last sentence (or change "must" to "should").

I have modified the documentation in v3 of the patchset to emphasize
that `in_end - in_start != 36` results in an error.

>
> >> +
> >> +/**
> >> + * Serializes a AVUUID into a string representation according to IETF RFC 4122.
> >> + * The string is lowercase and always 37 characters long, including the
> >> + * terminating NULL character.
> >> + *
> >> + * @param[in]  uu  AVUUID
> >> + * @param[out] out Pointer to a array of no less than 37 characters.
> >                                    ^
> >                                    n
> >
>
> Fixed.
>
>
> >> +/**
> >> + * Sets a UUID to nil
> >> + *
> >> + * @param[in,out]  uu  UUID to be set to nil
> >> + */
> >> +void av_uuid_nil_set(AVUUID uu);
> >
> > Why are these three functions not static inline? Exporting them seems
> > like a waste.
> >
>
> No particular reason, it's easy enough to change.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".